Skip to content

Fix illogical heading order for a single proposal#8877

Merged
andreslucena merged 2 commits intodecidim:developfrom
mainio:fix/proposals-illogical-heading-order
Feb 24, 2022
Merged

Fix illogical heading order for a single proposal#8877
andreslucena merged 2 commits intodecidim:developfrom
mainio:fix/proposals-illogical-heading-order

Conversation

@ahukkanen
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen commented Feb 22, 2022 •
edited
Loading

🎩 What? Why?

The proposal's title is in illogical order in context to the page title <h1>. According to the current layout, the logical order should be a <h2> in context to the previous headings on the page.

The problem can be seen e.g. on this page where the heading jumps from h1 -> h3:
https://meta.decidim.org/processes/roadmap/f/122/proposals/16935

Many screen reader users users glance the page quickly by going through its headings in which case this can be confusing.

WCAG 2.2 / 1.3.1 Info and Relationships (Level A)

https://www.w3.org/TR/WCAG22/#info-and-relationships
https://www.w3.org/WAI/WCAG22/Understanding/info-and-relationships.html

📌 Related Issues

Testing

Give the linked blog page to a visually impaired user and ask them to read it.

📋 Checklist

  • ❓ CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • ❌AVOID breaking the continuous integration build.
  • ❌AVOID making significant changes to the overall architecture.

@ahukkanen
Copy link
Copy Markdown
Contributor Author

ahukkanen commented Feb 22, 2022

The broken accessibility test for the single proposal page should be fixed after #8876 is merged.

@ahukkanen
Copy link
Copy Markdown
Contributor Author

ahukkanen commented Feb 22, 2022

I just noticed that the process subtitle is marked as <h2>, so the current order is not illogical in the context of that page.

But it is illogical in the overall context because e.g. on this page the debate title is marked as <h2> which makes more sense to me at least:
https://meta.decidim.org/processes/RedesignDecidim/f/1663/debates/234

@ahukkanen ahukkanen mentioned this pull request Feb 23, 2022
Merged
12 tasks
@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Feb 23, 2022

I just noticed that the process subtitle is marked as <h2>, so the current order is not illogical in the context of that page.

But it is illogical in the overall context because e.g. on this page the debate title is marked as <h2> which makes more sense to me at least: meta.decidim.org/processes/RedesignDecidim/f/1663/debates/234

This is what I see with the WAVE structure feature:

image

I agree that the resource title (in this case, the proposal title) should be a h2.

The current h2.heading-small should be a p.heading-small, according to the HTML 5.1 spec that I found by looking a bit in Stack Overflow about subheadings semantics.

@ahukkanen
Copy link
Copy Markdown
Contributor Author

ahukkanen commented Feb 23, 2022

@andreslucena See also:
#8876 (comment)
#8893 (comment)

These issues (and the related discussion) are closely related.

@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Feb 23, 2022

These issues (and the related discussion) are closely related.

Yes, I was just arriving there, I'll leave the feedback at the discussion. Thanks for taking care of this issue!

@ahukkanen ahukkanen mentioned this pull request Feb 23, 2022
Merged
12 tasks
@ahukkanen ahukkanen marked this pull request as draft February 23, 2022 13:35
@ahukkanen
Copy link
Copy Markdown
Contributor Author

ahukkanen commented Feb 23, 2022

The participatory subtitle issue will be fixed by #8901.

After that is merged, we can merge #8876.

And finally after that, I'll merge this with develop and the broken accessibility test should be fixed.

Meanwhile, I marked this as a draft for the time being, before we get those PRs merged.

@ahukkanen
Copy link
Copy Markdown
Contributor Author

ahukkanen commented Feb 23, 2022

Actually it seems that if we want to keep the build green, we first need to merge the comments heading order change at #8876. I'll go ahead and mark that ready for review.

@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Feb 24, 2022

FYI #8876 is already merged

@ahukkanen ahukkanen marked this pull request as ready for review February 24, 2022 10:50
@ahukkanen
Copy link
Copy Markdown
Contributor Author

ahukkanen commented Feb 24, 2022

@andreslucena I just merged with the latest changes in the develop and CI is now green.

This is ready to be merged. #8901 also requires this to be merged first.

@andreslucena andreslucena merged commit 4d176c0 into decidim:develop Feb 24, 2022
@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Feb 24, 2022

This is ready to be merged. #8901 also requires this to be merged first.

Merged!

Can you please backport to v0.26-stable 🙏🏽? Thanks

@ahukkanen ahukkanen deleted the fix/proposals-illogical-heading-order branch February 24, 2022 15:26
ahukkanen added a commit to mainio/decidim that referenced this pull request Mar 2, 2022
@ahukkanen
Fix illogical heading order for a single proposal (decidim#8877)
d43b114
@ahukkanen ahukkanen mentioned this pull request Mar 2, 2022
Merged
12 tasks
@ahukkanen
Copy link
Copy Markdown
Contributor Author

ahukkanen commented Mar 2, 2022

Backport at #8950.

@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andreslucena andreslucena andreslucena approved these changes

Assignees

No one assigned

Labels

module: proposals team: accessibility type: fix PRs that implement a fix for a bug

Projects

None yet

Milestone

0.27.0

Development

Successfully merging this pull request may close these issues.

3 participants

@ahukkanen @andreslucena @alecslupu