🇫🇮Finland @simohell

benjifisher → credited simohell → .

benjifisher → credited 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.

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

simohell → created an issue.

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".

benjifisher → credited simohell → .

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

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:

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.

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.

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.

@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?

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.

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

benjifisher → credited simohell → .

benjifisher → credited simohell → .

benjifisher → credited simohell → .

If the parameter disablekb=1 how will the player comply with WCAG SC 2.1.1?

From the description I read that "Setting the parameter's value to 1 causes the player to not respond to keyboard controls." which sounds like a direct violation of 2.1.1. How is the player operable by a non-screen reader user with keyboard only if the player's keyboard setting is disabled?

Is the keyboard enabled behaviour in conflict with 2.1.4 by activating the keyboard commands without the player having focus ("Active only on focus" allows single key shortcuts in WCAG)?

@smustgrave About tests: I don't a lot of test with themes currently. And this is theme specific f.e. Olivero does this already, but uses a different script. How would we go about testing this?

benjifisher → credited simohell → .

benjifisher → credited simohell → .

simohell → created an issue.

Announcement works, but I would recommend following the UK gov style of adding more information to the plain "0/250" description part. A screen reader user can access the text but without context. Fe. Voiceover tells the user there is additional information available and accessing that outputs "0/250" - it woulg be helpful I think even just to add the word "characters" to give context.

I was very happy to see this issue posted as this has come up in our testing as well and was planning to create this issue myself.

I tested that latest MR indeed fixes the label issue (with VoiceOver) but I noted that there seems to be also one degradation. It seems that the info about CKEditor accessibility instructions have dropped away. In our accessibility testing we considered that piece information (alt-0 / option-0 for help) to be important for the user.

I think we should keep that piece of information announced as well.

Without the patch:

With the patch:

I was testing with several different types of fields - named Body, Corpse, Cadaver and Carcass. Corpse-field was formatted long text without summary.

This issue is somewhat related to 🐛 Edit Summary label is unclear to users Active as the summary/main text labelling is a special case of the problem.

We looked at this today at the end of Drupal a11y office hours and based on that I moved the aria attribute to the correct button element. This still doesn't have a test.

An example from Voice over rotor why the use of article-tags for each media can be a problem.

simohell → created an issue.

simohell → created an issue.

benjifisher → credited simohell → .

benjifisher → credited simohell → .

One more thing about showing/hiding content:
Since the decorative image does not have an alt text it might be a good idea to hide the default alt-text as well as the override field. In my opinion it would be better express the effect of marking an image decorative.

benjifisher → credited simohell → .

benjifisher → credited simohell → .

benjifisher → credited simohell → .

benjifisher → credited simohell → .

I commented the issue on long text summary https://www.drupal.org/project/drupal/issues/1562776#comment-15427067 🐛 Edit Summary label is unclear to users Active

We discussed this issue at the usability meeting 📌 Drupal Usability Meeting 2024-02-02 Active where we agreed that a simple text change is not a sufficient improvement. At the meeting were present @benjifisher @shaal @ simohell @worldlinemine @duncancm and @rkoller.

We came up with a suggestion to

1. wrap summary and full text into a fieldset
   (we noted that fieldset stlyles may be too sublte for proper accessibility, but that is another issue)
2. use field name as the label for the fieldset
3. label text areas as summary and full text

Summary and trimmed are terms that are used by field display add teaser is a standard view mode that implements displaying the summary and/or trimmed version.

Full text is referenced by the summary description but not present at form.

If we don't want to have the summary field always displayed, we can place it inside a details element. On content creation the details element should be open. On my opinion editing content it should be closed to save space if no summary text was added and open if a summary text was added.

benjifisher → credited simohell → .

This issue is important but has changed a lot over the years.

With Drupal 11 and several versions before that we can have had several fields in the same node with a summary each. The teaser metafore is no longer present. However I see that the label "Summary" is not properly connected to it's "parent" field whenever the summary is not closed. Having an option to close the summary also seems to be theme dependant.

This seems to be an accessibility issue as well as in at least some situation the user might not know wether the summary above or directly below the long text field is the on that summarises that specific field.

I guess the Summary label should always have some reference to the main field label.

I would expect this issue belongs to field_ui but is also relevant for a number of themes.

Attaching screenshots from Olivero and Claro.

benjifisher → credited simohell → .

Hi, Issue 📌 Field selection breaks conventions and increases cognitive load Needs review changes the type selection from radio buttons to links that are displayed as a grid.

Will this patch work with that version as well?

Hi, I'm setting this as "won't fix". It will be fixed when 📌 Field selection breaks conventions and increases cognitive load Needs review is completed. That issue changes the field type selection from radio buttons to links and has already passed usability review.

benjifisher → credited simohell → .

benjifisher → credited simohell → .

benjifisher → credited simohell → .

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2024-01-05 Active . That issue will have a link to a recording of the meeting.

For the record, the attendees at today's usability meeting were @AaronMcHale, @benjifisher, @emma-horrell @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.

The MR addresses the main points of the issue. Visual presentation has been improved. Some of the usability problems present in the current MR are being addressed by other issues and seen as out of scope for this issue. Keyboard navigation was tested roughly and the MR improves it's usability.

This issue would benefit from separate accessibility review.

Requested change

A small change request is to make description texts in both form steps visually consistent. At selecting the field type description is smaller that the label and in a dark grey colour. Selecting the option for field type uses same style for both the option text and the description. It is recommended to use same styles for text in both steps: larger text for label/option and a bit smaller maybe dark grey for description (like in the first step).

benjifisher → credited simohell → .

xjm → credited simohell → .

benjifisher → credited simohell → .

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2023-11-24 Needs work and 📌 Drupal Usability Meeting 2023-12-01 Needs work . Each issue will have a link to a recording of the meeting.

For the record, the attendees at today's usability meeting were @AaronMcHale, @anushrikumari, @benjifisher, @ckrina, @rkoller, @simohell and @worldlinemine.

We recommend using compact text for easier reading. The context will allow short text for options.

Images exceeding maximum dimensions
• Resize proportionally
• Reject

We were of the opinion it would make sense to swap places for maximum dimensions and minimum dimensions so that minimum setting is first. This will help the new resize setting to stand out better in it's context as well as make the commonly used minimum setting slightly more prominent. If this is difficult to include in this issue, it's ok to make this a follow up issue to be done later.

We also thought it worthwhile to consider using a fieldset for the "Images exceeding maximum dimensions" in similar manner to how /admin/config/development/settings shows the detailed options under the first checkbox when checked. It's ok to make this a follow up issue to be done later.

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

benjifisher → credited simohell → .

There are still some text changes coming from the usability meeting, I was on sick leave for a few days and failed to follow up. Sorry.

@keszthelyi it's a bit hidden, but you can get the patch directly from the MR clicking the text "plain diff"'

benjifisher → credited simohell → .

Regarding #240 there is widespread patterns of having the "show password" styled as a link. For instance USWDS
https://designsystem.digital.gov/templates/authentication-pages/sign-in - that should be accessible.
But I do see the problem with having an actual link close by...

Dropped testing for the attribute that got removed.

simohell → created an issue.

The latest screenshot shows help text that is outdated since field label update was about a week ago.

We might also consider to make the text a bit shorter.

- The options are mostly self explanatory, clear without help text.
- EXIF is a bit of excess detail. Usually if a person has use of EXIF there is already knowledge of how it behaves - I assume.
- If we want to give techincal info maybe link to image settings page where compression percentage. We could explain EXIF part there.
- Text could clarify that the resize method is scaling proportionally, not resizing to both dimesions.

benjifisher → credited simohell → .

It looks like the fix for current core layout was simpler than I thought.
I managed to test this right, all that was needed was setting a minimun value for main column:
not grid-template-columns: 3fr minmax(22.5rem, 1fr);
but instead grid-template-columns: minmax(0, 3fr) minmax(22.5rem, 1fr);

I have implemented that fix as a merge request in the other related issue 🐛 Long string breaks the layout of Claro Needs review .

It looks like the fix for current core layout was simpler than I thought.
I managed to test this right, all that was needed was setting a minimun value for main column:
not grid-template-columns: 3fr minmax(22.5rem, 1fr);
but instead grid-template-columns: minmax(0, 3fr) minmax(22.5rem, 1fr);

W3C's "HTML and XHTML Techniques for WCAG 2.0" has an example code for using a-tag without an href attribute and without text content (so obviously invisible to sighted users) - but it does not discuss presentation in case there would be some text. An h-tag linked from an a with href in that document is not styled differently from other headings though. This feels like a contradiction to what is said in #23.
https://www.w3.org/TR/WCAG20-TECHS/html.html#H86-ex2

There is a case of links without hrefs though. It is possible to have an <a role="link">text</a> or more commonly an image or a span. Semantic HTML is however considered best practise.

I could bring this up during tomorrows accessibility office hours if somebody else doesn't want to do it.

If the issue exists all the way to the latest dev we fix it there and then bring the fix from there to the other versions as well.

Adding screenshots from different versions in different sizes.
One is narrow, second is about FullHD, third is about full 27" 1440p monitor.

I think we have currently 3 options:
- fill the vieport width with form
- set maximum width and center form
- set maximum width and center main columns, fix side column to side of the window

(additional note: how does this work with RTL-languages?)

I will update screenshots with Umami demo-content in a while to give better comparison of actual use case.

@amanire This latest version seem to work and also fixes 🐛 Long string breaks the layout of Claro Needs review

It might be a bit better to double the gap and allow side bar to grow a bit more in a larger screen, but that needs maybe a bit more eyes to decide. This fixes the bugs is good enough I think for now. I like having the sidebar stay in the middle but that is a bit bigger change to the current version so I hope to hear what @ckrina thinks.

I tagged this for usability review and hope to have this in the meeting next Friday.

I also added your screenshots and a new one with some clipping taking place.

Maybe also update the text description?

Ah. But it's not a regression by this issue, so this would be improvement as such. I noticed that you updated the other MR so I'll check that one and compare results. This one fixes one single bug, the other one maybe more. Changing only one thing was an idea to get it merged quickly but let's see.

The current MR does not add any hard-coded widths, so maybe you are testing an older commit?

width: 100%;
max-width: 52rem;

Perhaps the old CSS is cached in your setup. Or you have some custom plugin or forced CSS that overlaps here.

With an extremely long string the width may increase for Firefox and Safari but without the change the whole form is pushed away from viewport. Current dev branch does have certain viewport widths where some browsers clip the sidebar a bit, but that is not caused by this MR.

Ok. I updated the CSS so, that this fix does not create regression to Firefox and Safari.

Firefox and Safari continue to have certain clipping in some viewport widths, but not more than with the current production version. FF and Safari both also benefit from this fix even if other add/edit form layout issues still affect those browser.

I think this small fix is ready to be merged and the other browser specific issues could be handled in 🐛 Layout Issues with Claro theme Needs work .

Testing this MR I see that the main column expands without limit.
Expanding a text area without a limit harm usability on large screens.
Studies show that optimal width is around 45-75 characters/line.

The solution in #3400762 was chosen as the only change it was supposed to make was fixing the bug, but unfortunately it was not tested on all browsers and some browsers render it in a way that was not intended.

I think we need to find the CSS to keep the form in a usable width while making the form keep from breaking up.

I'll try to tag @ckrina in Slack on this to check if there how much space there is to iterate on the layout or just to fix the actual bugs.

simohell → created an issue.

The bug mentioned in comment #37 will be fixed in 🐛 Long string breaks the layout of Claro Needs review  with a minimal CSS change.
The patch here deals with the piece of CSS, so it would make sense to recheck this issue against that code.

I updated the 3 tests for word "dimensions".

I had deliberately left out changing the "high-resolution" part since it makes sense if read according to for instance Adobe's definition
"High resolution images are pictures or photos where the media has higher concentrations of pixels or dots, resulting in better quality and clarity of the image – as it contains more detail."

A number of end users deal with high-resolution photographs and the help-text would make sense to them. In the past the industry used terms like "press quality", "print quality" or "web quality" and here press & print would be high-resolution and web quality low-resolution. The help text communicates that Drupal handles original high-resolution photos just fine.

Functionality not changed since previous review

Quite right. Missed that one, but now it should be ready.

benjifisher → credited simohell → .

The patch changes the UX significantly for large screen and is out of scope for this issue. Very wide textareas are usually considered bad UX.
I working on this at the moment at Helsinki contrib event and my current goal is to fix the bug without changing the layout as a whole.