Account created on 5 December 2007, almost 17 years ago
#

Merge Requests

More

Recent comments

🇫🇮Finland simohell

This change is straightforward in code and fixes the issue on an actual site that has the problem with background color.

🇫🇮Finland simohell

Canvas is a system color so it varies. On my site and my browser it is white. It should be same colour as html file with no css or color attributes. With CSS 2 it would have been "Window" but that is deprecated in CSS 3, I think.

🇫🇮Finland simohell

I have the issue and it seems that the issue has something to do with the body background colour not being defined. This means that body is transparent and actually relies on the browser/os setting the correct default colour.

When the body content and styles are rendered on top of another element that has set a background colour it shows through.

The preview should use as the highest level content content (or it's parent):
background-color: Window;

See: https://www.w3.org/TR/css-color-3/

🇫🇮Finland simohell

New patch for 10.3.6 due to the 1 line added to core CommentLazyBuilders.php

🇫🇮Finland simohell

Navigation between components

Default grid navigation is done with arrow keys and for accessibility the area navigated with arrows may be limited.
Tab is used for navigating between components.

With Experience Builder the structure may be wild and deviate from a basic grid a lot. This is due to possibility of complex nesting.

Experience Builder has the layer view that is a well formed hierarchial list. For this keyboard navigation can well use standard patterns moving between components. We should utilise this view as an keyboard usability/accessibility feature and allow accessing component properties from here easily. While testing an older version of XP the layers and the layout view were both highlighted when navigating in the structure. Using layers to navigate components should activate highlighting in the layout area, but in a way that the user is not confused where the keyboard focus actually is (I'd maybe use some shading overlay with active component with normal brightness, but did not look into current best practices).

For navigating a complex layoutdirectly in the layout view there are tradeoffs required. It might not be a straight grid. Following standards might not reult in fulfilling user expectations. I have a few example cases where I think user expect different focus change to that which structure would provide. See the attached PDF, ask for more detail.

🇫🇮Finland simohell

Copy/Cut/Paste

Here it's best to use the usual Ctrl-C, Ctrl-X, Ctrl-V

For duplicating common would be Ctrl-D but here (because of nesting) it is not clear what it should do. Eg. if one is inside a component that has 1 slot and duplicates the component in the single slot, what would happen with duplicate?

While pasting an improved focus highlight is needed in addition to understandable keyboard navigation. Pasting a component could limit navigation to component slots, but to exit the paste mode we need a shortcut to clear clipboard. C for clear, F for Flush and R for reset are not available as shortcuts. A fair option could be Ctrl-Alt-Z since action is an undo of a sort (even if that keyboard is used by some Google apps for accessibility features)

🇫🇮Finland simohell

I this passes for usability, there probably needs to be additional code for accessibility. This would add several links with the same text, so it would be good to have aria-label to always include the actual name of the page in question (WCAG 2.4.4).

🇫🇮Finland simohell

There are similar findings from user testing. We however don't know what is the best way to tackle the issue.

This is also related to a bigger discussion on wether or not we benefit from such overview-pages in admin path structure. This comes up every once in a while in UX discussions when we are moving pages from one top level menu to another.

🇫🇮Finland simohell

Initially I thought that I'm against the idea at #21. I was testing this today for the first time on my own, and I thought that the layer tab is a very good tool for power users and people who need accessible technologies - and this is especially because of being visually cleaner.

But if it would be a per user setting to show or hide previews then it would be an improvement. Another improvement IMO would be to allow navigating from layers to props and back using the keyboard.

Relevant innovations and lessons learned in dragging would be welcome at 📌 Research accessibility of drag-and-drop grid interfaces. Active

🇫🇮Finland simohell

This issue has a script for navigating field types with keyboard in a grid style. At the moment it is set to "needs work" with recommended changes, but might be worthwhile to check 📌 Grid-style field type keyboard navigation Needs work

🇫🇮Finland simohell

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2024-09-20 Needs work . That issue will have a link to a recording of the meeting. For the record, the attendees at last Friday's usability meeting were @AaronMcHale, @anmolgoyal74, @benjifisher, @rkoller, @shaal, @simohell, and @zetagraph.

If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.

Recommendations

Use radio-buttons to show both the options as equal choices. This will be clearer for the user and there even is an edge case, where a Safari browser supports lossy format but not lossless. Label them “WebP quality” to match JPEG quality as equal section.

WebP quality
- Lossless compression
- Lossy compression

As "Lossy compression" is selected display the numeric field indented visually to the same level with the radio-button.

WebP quality
- Lossless compression
- Lossy compression
[ 75% ] Set the image quality for WebP manipulations. Ranges from 0 to 100. Higher values mean better image quality but bigger files.

The label text is a suggestion, it was not decided by consensus in the meetiong.

It is also suggested to change the word “define” to “set” in the compression settings description.

Updating documentation

Include the information about WebP setting into documentation at https://www.drupal.org/docs/7/core/modules/image/working-with-images#s-s...

Add a note, that in some old browsers or operating systems versions it may be, that a browser supports the lossy compression but not the lossless compression.

🇫🇮Finland simohell

For navigation between/within layout element, I'm pretty sure we should use something like "roving tabindex" to have less tabbing and more arrow keys.

Saw a video on Youtube and now I'm sold ;-)

https://www.youtube.com/live/lLrmnFJKFAM?feature=shared

🇫🇮Finland simohell

On the subject of checkbox vs button and old issue has a 9 year old comment #36 that is still IMO valid: "The functionality is already in Core, it's just about applying this consistently across the UI."

The functionality is in admin/content, nut missing from other similar lists.
I think we should aim to make the functionality available "everywhere" and decide on a unified style of UI for it across the admin UI.

🇫🇮Finland simohell

In comment #7 it was said, that this issue is a definition issue (It all comes down to the interpretation of "User Interface Component" ), while accessibility is for people actually using it. I would like to get an actual user story about when the proposed change increases accessibility instead of reducing it. Making Drupal harder to use for people for keyboard only users just to check a box in a form does not seem right, if there is no actual accessibility benefit.

It seems to be incorrectly stated in #9 that disabling keyboard support would be required for screen reader support. VoiceOver I think has no issues with Youtube while JAWS, NVDA and Narrator have built in commands to start using the player with keyboard shortcuts active. With the screen reader in such mode active should not result in any keyboard conflicts.

WCAG does however state that with voice input there may be issues with single letter commands due to background noise. Not sure how to tackle this.

(I guess the best person to get a comprehensive answer to this would be Rain who is all of Drupal Accessibility Maintainer, Google Staff UX Design Manager and W3C Accessibility Guidelines Working Group Member)

🇫🇮Finland simohell

Not really helpful for fixing this issue I guess, but I made a version of MR with suggestions applied into a patch for 10.2.x. If someone else finds it useful to take a head start before this comes into core.

🇫🇮Finland simohell

It looks like Wix doesn't have a full keyboard support, since some shortcuts say fe. "while dragging" https://support.wix.com/en/article/wix-editor-keyboard-shortcuts-in-the-...

🇫🇮Finland simohell

To keep in mind:
- avoid conflicts with users accessibility technology (https://www.w3.org/WAI/WCAG21/Understanding/character-key-shortcuts)
- about navigating grids, includes examples of common keys (https://www.w3.org/WAI/ARIA/apg/patterns/grid/#keyboardinteraction-setti...)
- navigation matching html-element and visual (eg. no radio-button grids with directional arrows, if non-visually only prev-next)

🇫🇮Finland simohell

Merge request code is from @kala4ek patch in 🐛 RuntimeException: Adding non-existent permissions to a role is not allowed Active and idea of adding log message from @Jaswinsingh at the same thread.

For UX review issues:
- this issue arises from fault in other modules. Is it wise to hide the errors caused by them from the user?
- there is a manual solution to the issue https://www.drupal.org/project/rip
- which is better for the user, automatic or manual solution?
- does logging add value? should it be silent or maybe a status message?

🇫🇮Finland simohell

simohell changed the visibility of the branch 3464893-faulty-configuration-results to hidden.

🇫🇮Finland simohell

Sorry for having to wait for a reply.
I just had the at that moment latest 11.x dev branch of core, nothing else.
And the other one was the this issues branch.

🇫🇮Finland simohell

Let's check the approach (automated removal vs. manual drush command) and the log message at a UX meeting.

🇫🇮Finland simohell

I used patch from #36 succesfully.

There are scripts, including the module https://www.drupal.org/project/rip , but since these errors result in a WSOD I think it is preferabe to have an automated Core solution for it.

🇫🇮Finland simohell

The current MR seems to add the checkbox twice on admin/content - once before and once after the toggle for in a narrow view toggling on and off the low priority column. The order of the two toggles is something to be decided. The toggle seems to be missing admin/content/blocks, admin/content/files or admin/content/media - the last of which also has an alternative grid-view that doesn't need one.

On permissions page the checkbox is also doubled, but there are no other controls in between.

I think we should have do both the toggles with similar implementation. Now one is a checkbox and the other is a button. Styling can then be decided per theme if one would like to use a pin-icon for sighted users etc.

🇫🇮Finland simohell

Here is a new article about the topic:

Exploring the challenges in creating an accessible sortable list (drag-and-drop) by Kenfall Gassner from Girhub

https://github.blog/2024-07-09-exploring-the-challenges-in-creating-an-a...

🇫🇮Finland simohell

As this was tagged, I had this queued for UX review meeting on May 10 but while preparing, I couldn't reproduce the original usability issue as it was already fixed as mentioned in #32. We didn't therefore do a formal review/recommendations and the related discussion is not included in the meeting recording.

🇫🇮Finland simohell

I applied a fix for deprecated Drupal::entityManager() to patch from #98: Show complete thread below reply/preview form and tested that it applies to 10.3.1. I didn't test the functionality yet, though.

🇫🇮Finland simohell

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2024-07-05 Needs work . That issue will have a link to a recording of the meeting. For the record, the attendees at today's usability meeting were @benjifisher, @rachelh_design, @rkoller, @shaal, and @simohell.

If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.

We reached a consensus, that the best option for now, is to allow machine names to break word, but disable hyphenation.

We recommend limiting the scope to this single change and leave the rest for existing functionality and rendering implemented by each browser. For future, in a much wider context, we could consider using for mobile/narrow viewport a structure different from html table to improve usability.

🇫🇮Finland simohell

On some occasions on 10.3 logged in as admin with #221 also gives me

VM890 facets-views-ajax.js:202 Uncaught TypeError: Cannot read properties of undefined (reading 'facets_summary_ajax_summary')
    at updateFacetsSummaryBlock (VM890 facets-views-ajax.js:202:36)
    at updateFacetsBlocks (VM890 facets-views-ajax.js:179:9)
    at updateFacetsView (VM890 facets-views-ajax.js:154:5)
    at HTMLUListElement.<anonymous> (VM890 facets-views-ajax.js:89:17)
    at HTMLUListElement.dispatch (VM835 jquery.min.js:2:40035)
    at v.handle (VM835 jquery.min.js:2:38006)
    at Object.trigger (VM835 jquery.min.js:2:70124)
    at HTMLUListElement.<anonymous> (VM835 jquery.min.js:2:70726)
    at Function.each (VM835 jquery.min.js:2:3129)
    at ce.fn.init.each (VM835 jquery.min.js:2:1594)

And after that stops accepting checking facets from the same group (I have several taxonomies with their own sets).
It seems to work for anonymous, however.

🇫🇮Finland simohell

Patch from #221 fixes the functionality for me. I am however getting

Uncaught TypeError: Cannot set properties of undefined (setting 'closeMessage')
at messages.js?v=10.3.1:54:27
at messages.js?v=10.3.1:55:3

on each click, but so far I don't see it causing any problems.

🇫🇮Finland simohell

But this apparently was fixed in Drupal 10.3 about a month ago - which is great :-)

🇫🇮Finland simohell

@clau_bolson the case for @rossidrup was different, since the view appears to have commerce products. I have the same issue and I'm looking for solutions.

🇫🇮Finland simohell

I think we should expand the scope to include also the Label column.

The label column does not break lines at all while the machine name which needs to be an exact reference does. It is not common to have very long words in labels, but in some languages and some edge cases there may be fairly long words.

The text seem to jus inherit hyphens: auto from body.
word-break property seems no to be set at all.

For the machine names we would want to have them readable as is an also copy them for pasting elsewhere. It is a common use case that the start of the machine name is the same for several fields and the end is significant. This speaks against having an ellipses. Thus I suggest adding

word-break: break-word;
hyphens: none;

for the machine name (it's always a single word anyway)

For the label I think it would make sense to have word-break: break-word; but keeping the default auto-hyphens.

I wonder if it would merit to have a separate class to handle the unhyphened breaking words so that it would be reusable where need arises? Or would that be an overkill?

🇫🇮Finland simohell

The new issue to re-fix this is ready for review.

🇫🇮Finland simohell

I asked @laurii and @xjm in slack admin UI about due process to fix this. Based on Lauri's the answer I will open a new issue.

🇫🇮Finland simohell

Oh. Sorry, that "0/250" case was using a custom setting. So it's not something with the module / patch. This looks great.

🇫🇮Finland simohell

I have now tested this on Drupal 10.3.x (with Umami-demo) adding a video embed field to the article content type. I embed a Youtube video with English subtitle to a piece of content.

With unpatched 2.x branch:
I navigate to a video by keyboard
I set the subtitles on
I set the subtitles auto-translation on to German
I see German subtitles

WIth patch applied
I navigate to a video by keyboard
I set the subtitles on
I navigate to subtitles configuration option
I am blocked from accessing the settings

This patch therefor breaks modules existing support for WCAG SC 2.1.1.

🇫🇮Finland simohell

I have retested Youtube player for keyboard accessibility with disablekb=1 setting on.

Please see the linked video recording on how Youtube Player with disablekb=1 setting breaks keyboard navigation on the subtitles, translation and quality settings. First part of the video has default keyboard behaviour and the second part has disablekb setting on and some of the settings are totally inaccessible to a keyboard user.

https://youtu.be/rfXD7bXQRQg

I did not actually test with a Drupal installation but it uses the Youtube player, right?

🇫🇮Finland simohell

Please attach the reviews you have, as they contradict the short discussion (not a formal review) at Drupal's Accessibility teams #accessibility channel on April 4 this year. The understanding there was, that the Youtube player is not in breach of SC 2.1.4 with the default keyboard option.

For clarification could you give an example of a real use case where the players default keyboard controls cause accessibility problems? It would very much help to understand the propose solution.

🇫🇮Finland simohell

Based on my tests (Chrome on Mac OSX) adding "disablekb=1" would partially break WCAG SC 2.1.1 resulting in violating also WCAG SC 1.2.2 for keyboard users by making it impossible for the user to choose captions.

According to Bureau of Internet Accessibility the Youtube player should be accessible in regard to keyboard use (https://www.boia.org/blog/are-embedded-youtube-videos-bad-for-accessibility). This was also the understanding on the Drupal accessibility channel earlier.

How did the two accessibility teams solve the issue of managing captions on keyboard if "disablekb" is on?

🇫🇮Finland simohell

Since 1. was her the part affecting usability and was fixed already and the remaining task for this ticket is the test, I'm removing "usability" and needs usability review" tags. If we need to look at the usability aspects of the message (such as order of the recommendations, security updates on each supported minor etc.) it should prbably be a new issue.

Attaching an example screnshot for 10.1.1 message for reference.

🇫🇮Finland simohell

I created an issue for marking deprecated/experimental themes: 📌 Mark deprecated and experimental themes on admin appearance page Active

🇫🇮Finland simohell

From #20 the task "Either a Claro maintainer or (preferably) a frontend framework manager needs to validate the approach." I think falls under the definition of "review" so I'll tag this back to "needs review".

🇫🇮Finland simohell

Sorry. I had both but comment had 11.x HEAD only as attachement, not inline.

🇫🇮Finland simohell

Including /core/modules/system/css/system.admin.css would break some of the styling in Claro, so it makes sense that same icon is added to Claro in it's own CSS. Attached a screenshot of what would happend if the system CSS file would be used. #12 shows the expected result.

Claro with system.admin.css applied:

🇫🇮Finland simohell

Now I have a Nightwatch test that uses Claro theme for the test site and fails against 11.x without and succeeds against this MR. So even if it's using much of the test code from Olivero it now actually tests the menu in Claro.

Attaching both the test results run locally.

🇫🇮Finland simohell

Oh. Now I see that that test actually runs again an Olivero install.
So I'll need to make it run with actual Claro. My bad, need to fix it.

🇫🇮Finland simohell

We agreed to add the duplicate nightwatch test as it is adding a comment with @lauriii during DrupalCamp Finland+Baltics contrib day - since locally it passed and we didn't find examples of nightwatch tests testing multiple themes/modules.

🇫🇮Finland simohell

@xjm You mean the main issue is, that 11.x does not include /modules/system/css/system.admin.css on admin pages on Claro (or Olivero for that matter) even if it should? And it is also not certain wether it should or not?

🇫🇮Finland simohell

I reviewed this issue, that is a follow-up on #3215043: Indicate the non-stable statuses in admin/modules page .

This MR adds an existing icon from the theme in front of the link with text "(Deprecated)". This icon used is the correct one and helps user to identify that using a deprecated module carries some risk. Icon is added as a background image and does not have effect on Voiceover screenreader.

The confirmation message does not need the additional icon as the whole message is styled as a warning that has it's own icon.

🇫🇮Finland simohell

Having a separate column makes it easier to visually scan labels and machine names. I suppose there are pros and cons to both ways.

Production build 0.71.5 2024