Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BITV] No heading for settings pages #35626

Closed
5 of 9 tasks
Tracked by #33744
ChristophWurst opened this issue Dec 6, 2022 · 10 comments · Fixed by #36640
Closed
5 of 9 tasks
Tracked by #33744

[BITV] No heading for settings pages #35626

ChristophWurst opened this issue Dec 6, 2022 · 10 comments · Fixed by #36640

Comments

@ChristophWurst
Copy link
Member

⚠️ This issue respects the following points: ⚠️

  • This is a bug, not a question or a configuration/webserver/proxy issue.
  • This issue is not already reported on Github (I've searched it).
  • Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
  • Nextcloud Server is running on 64bit capable CPU, PHP and OS.
  • I agree to follow Nextcloud's Code of Conduct.

Bug description

If I open the settings page of Nextcloud there is no h1 heading found in the page's source.

Steps to reproduce

  1. Open /index.php/settings/user or /index.php/settings/admin
  2. Navigate through the pages from the left sidebar

Expected behavior

A h1 in the content area for the currently selected entry, e.g. "Personal info".

The heading should have tabindex="-1" so the focus jumps to it after selecting the page.

Installation method

Official All-in-One appliance

Operating system

RHEL/CentOS

PHP engine version

PHP 8.1

Web server

Other

Database engine version

MySQL

Is this bug present after an update or on a fresh install?

Fresh Nextcloud Server install

Are you using the Nextcloud Server Encryption module?

Encryption is Enabled

What user-backends are you using?

  • Default user-backend (database)
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Configuration report

No response

List of activated Apps

N/a

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

No response

@ChristophWurst ChristophWurst changed the title [Bug]: No heading nor tab index for settings pages [BITV]: No heading nor tab index for settings pages Dec 6, 2022
@ChristophWurst ChristophWurst changed the title [BITV]: No heading nor tab index for settings pages [BITV] No heading nor tab index for settings pages Dec 6, 2022
@ChristophWurst ChristophWurst moved this to 📄 To do (~10 entries) in 💌 📅 👥 Groupware team Dec 13, 2022
@ChristophWurst
Copy link
Member Author

Bildschirmfoto vom 2022-12-14 16-47-14

there is already an h1 and the sections on the settings pages use h2

@ChristophWurst
Copy link
Member Author

ChristophWurst commented Dec 15, 2022

So currently we have a rather generic hidden h1 and h2s for the settings sections.

I would propose to drop the hidden h1 for this page and instead add a visible h1 with the settings page name on top of the h2 sections.

Does that sounds ok from a design perspective @nimishavijay and from an accessibility perspective @michaelnissenbaum? Is this change a requirement for certification?

@michaelnissenbaum
Copy link

From accessibility perspective it doesn't make any difference whether heading is hidden or visible. Important is whether the structure of the headings makes sense in regard to the content of the page.

@nimishavijay
Copy link
Member

I would propose to drop the hidden h1 for this page and instead add a visible h1 with the settings page name on top of the h2 sections.

So for eg. when you are in the Security section, the top of the content will have an h1 "Security", and h2 "Two factor authentication", "Passwordless authentication", "End-to-end encryption" etc, did I understand that correctly? That sounds good to me.

@ChristophWurst
Copy link
Member Author

Yes, that is how it would render

@JuliaKirschenheuter JuliaKirschenheuter self-assigned this Feb 7, 2023
@JuliaKirschenheuter
Copy link
Contributor

JuliaKirschenheuter commented Feb 8, 2023

Headings have to be fixed in:

  • Files
  • Personal + Administration settings
  • Users
  • Apps
  • Photos App
  • Activity App

@nickvergessen
Copy link
Member

Personal + Administration settings

Most of them use the settings component we have and that has headers in them 🤔

@JuliaKirschenheuter
Copy link
Contributor

Most of them use the settings component we have and that has headers in them

I understand what you mean. I've understood this ticket this way: somehow it should be possible to create a individual name for each page from the sidebar. For now there only headings for for example "Version", "Update" and so on available. But how is it possible to identify "Overview" which is inside of "Administration"?

Screenshot from 2023-02-10 16-07-45

@nickvergessen
Copy link
Member

Well that block is not using vue yet, so you need to do it directly in it's template:
https://github.com/nextcloud/server/blob/master/apps/settings/templates/settings/admin/overview.php#L31

@JuliaKirschenheuter
Copy link
Contributor

JuliaKirschenheuter commented Feb 13, 2023

There is one acceptable solution which will set right <h1> from page to page and won't break existing page layout: existing <h1> have to be replaced with current value from $_['pageTitle']. Because of it it is important to set an initial value for pageTitle in each app: Users, Apps, Activity, Photos, Personal/Admin settings and Files. This is already done via setting initial value for additional param pageTitle in TemplateResponse of each responsible Controller:

TODO:

In addition to initial value of pageTitle it is need to update it relating to current page. In all apps besides Personal/Admin settings browser history API takes place. Because of this a function which implements an update of pageTitle should be written in https://github.com/nextcloud/nextcloud-router. This function should be called:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants