backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 38 forks source link

[UX] User account email settings: fieldsets use for #group a value that does not match any form element #6066

Open kiamlaluno opened 1 year ago

kiamlaluno commented 1 year ago

In user_settings_email() the fieldsets use '#group' => 'email' which would make them vertical tabs, but that form does not contain any $form['email'] element whose type is vertical_tabs. The effect is that those fieldsets are rendered as normal fieldsets, on /admin/config/people/emails.

screenshot

This does not happen in user_admin_settings(), where the same fieldsets are rendered as vertical tabs. (The screenshot has been taken from a Drupal 10 test site.)

screenshot

Since on /admin/config/people/emails there are just those fieldsets, it does not make sense to show them as vertical tabs. The solution for Backdrop is removing from user_settings_email() all the '#group' => 'email', lines.

(I changed the second screenshot with one showing the full page on a Drupal 10 site.)

kiamlaluno commented 1 year ago

I am going to create a PR, once I found out how to use the GitHub interface a browser extension modified. 😅

klonos commented 1 year ago

Since on /admin/config/people/emails there are just those fieldsets, it does not make sense to show them as vertical tabs.

🤔 ...I agree with this in principle, but not 100% sure that having all these settings be in fieldsets instead of vertical tabs is ideal UX. With fieldsets, it takes 2 clicks to move between each email setting (1 click to collapse the current fieldset - a 2nd click to expand the next), whereas with vertical tabs it only takes a single click.

olafgrabienski commented 1 year ago

@klonos Good point! I agree that vertical tabs would be a much better UX for this special form with its many and long text(area) inputs.

klonos commented 1 year ago

Yes. The Drupal UI guidelines say specifically for vertical tabs that they should be used to group together form elements of less significance, that can usually be left to their defaults and ignored - they should not be used for the main interaction of the form. That's why I started my previous comment with a 🤔 emoji. However, I just had a look at the guide again, and it says the following:

Use vertical tabs:

  • when you have a group of form elements that users can safely ignore, such as metadata elements or elements with good defaults; or
  • for situations similar to the Emails settings on the Accounts settings page.

So @kiamlaluno I think that we should better change those fieldsets to be vertical tabs, since:

Pinging @jenlampton about this as well.

kiamlaluno commented 1 year ago

Vertical tabs are used for optional settings that normally are not changed. That is why user_admin_settings() uses vertical tabs only for the last part of the form settings and not the full form.

For what I can see in Drupal 7 settings pages, vertical tabs are not used on a setting form that shows just them.

kiamlaluno commented 7 months ago

I have updated the MR. The failing test is the usual Layout Upgrade Test.

olafgrabienski commented 7 months ago

@kiamlaluno Thanks for updating the pull request. Do you think fieldsets are more appropriate than vertical tabs here, even if vertical tabs are recommended "for situations similar to the Emails settings on the Accounts settings page"?

kiamlaluno commented 7 months ago

The guide on drupal.org says also:

Do not use vertical tabs for the main interaction in the form.

Do not use more than 9 vertical tabs, because it takes up too much vertical space.

Don't use a pane that is too long. Vertical tabs are meant to be in view with the content of the page to allow orientation.

It also says for situations similar to the Emails settings on the Accounts settings page, but the account settings page does not contain just vertical tabs. With Drupal, on /admin/config/people/accounts, there are also six field-sets (Anonymous users, Contact settings, Administrator role, Registration and cancellation, Personalization, and Privacy). That means they are still following what the guide says, since they are not using vertical tabs for the main interaction in the form.

klonos commented 1 month ago

With Drupal, on /admin/config/people/accounts, there are also six field-sets ... That means they are still following what the guide says, since they are not using vertical tabs for the main interaction in the form.

For reference, here's a screenshot of that: image

klonos commented 1 month ago

FWIW, although I agree with the design guide in general re vertical tabs, I believe that an exception should be made in this case in Backdrop. I will bring this up during the next design/UX or dev meeting to see if we get a consensus about it.