Account created on 5 December 2007, over 16 years ago
#

Merge Requests

More

Recent comments

🇫🇮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.

🇫🇮Finland 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)?

🇫🇮Finland simohell

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

🇫🇮Finland simohell

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.

🇫🇮Finland simohell

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.

🇫🇮Finland simohell

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.

🇫🇮Finland simohell

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

🇫🇮Finland 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.

🇫🇮Finland 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

🇫🇮Finland simohell

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.

🇫🇮Finland 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.



🇫🇮Finland 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?

🇫🇮Finland simohell

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.

🇫🇮Finland 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).

🇫🇮Finland 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.

🇫🇮Finland 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.

Production build 0.69.0 2024