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

Center the FAQ lists #3255

Merged
merged 3 commits into from
Nov 29, 2022
Merged

Center the FAQ lists #3255

merged 3 commits into from
Nov 29, 2022

Conversation

MikeMcC399
Copy link
Contributor

This PR resolves the enhancement request #3246 "FAQ Sections is not centered".

The lower section of https://www.coronawarn.app/en/faq/ is now centered so that it has a consistent look independently of the available browser width.

Old


Previously
image

New


Desktop wide
FAQ wide


Desktop narrow
FAQ narrow


Mobile
FAQ mobile

@MikeMcC399 MikeMcC399 requested a review from a team November 28, 2022 12:06
@brianebeling
Copy link
Member

Hey @MikeMcC399

Thanks for the change. Unfortunately I'm unable to review it today as I would really like to take a more in-depth look.
I'll try to review and test it tomorrow.

Specifically I'd like to review the centered title and see if we can find some alternatives. But I also want to take the opinion of others into consideration if possible.

@MikeMcC399
Copy link
Contributor Author

@brianebeling

Please take your time. There is no urgency from my side.

If it is desired to have the "Sections" title on the left again that would be an easy change (just move text-left from line 31 up to line 29).

As the screen gets narrower it is more noticeable how much white space is used. I haven't changed this.

If you want to do further layout optimisation it might be an idea to create a separate non-fork branch instead of master to merge this PR into.

@brianebeling brianebeling self-assigned this Nov 28, 2022
@mtb77
Copy link
Member

mtb77 commented Nov 28, 2022

@brianebeling just tell me when its ready to review :)

@mtb77 mtb77 self-requested a review November 28, 2022 14:19
@brianebeling
Copy link
Member

@MikeMcC399 This might be a bit of a wild idea, but what if we remove the Sections title? I don't think it adds any value. Just looking at the table already suggests these are the categories or sections available.

@brianebeling
Copy link
Member

image
So basically this

@MikeMcC399
Copy link
Contributor Author

@brianebeling

  • I'm afraid that removing the "Sections" header is just scratching at the surface of issues on this page. It removes an <h1> tag which would then be no longer available for screen reader navigation.
  • I also wonder why the text "View all topics and questions" at the top shows all FAQs articles whereas "View all topics and questions" at the bottom of a column shows only the topics from the column. Why isn't the text at the top more prominent if it is more important? Why doesn't it have an <hn> tag? Why is this link closer to the search box when it belongs more to the bottom section?
  • The fact that the display of some titles in the columns is suppressed, but then they show up in the results page is non-intuitive. If they have a lower priority then they should be shown visually with a lower priority.
  • Then there is the issue of how many items are displayed in the column of lists and where it fades out.

I think it would be possible to spend quite a bit of time investigating and optimising these and related points.

For the moment I would restrict this PR to the centering topic only. If you want me to put the "Sections" header back on the left again, let me know and I will update the PR. I would however not want to remove it as an isolated change to this page.

@brianebeling
Copy link
Member

brianebeling commented Nov 29, 2022

Brian-J. Ebeling

  • I'm afraid that removing the "Sections" header is just scratching at the surface of issues on this page. It removes an <h1> tag which would then be no longer available for screen reader navigation.
  • I also wonder why the text "View all topics and questions" at the top shows all FAQs articles whereas "View all topics and questions" at the bottom of a column shows only the topics from the column. Why isn't the text at the top more prominent if it is more important? Why doesn't it have an <hn> tag? Why is this link closer to the search box when it belongs more to the bottom section?
  • The fact that the display of some titles in the columns is suppressed, but then they show up in the results page is non-intuitive. If they have a lower priority then they should be shown visually with a lower priority.
  • Then there is the issue of how many items are displayed in the column of lists and where it fades out.

I think it would be possible to spend quite a bit of time investigating and optimising these and related points.

For the moment I would restrict this PR to the centering topic only. If you want me to put the "Sections" header back on the left again, let me know and I will update the PR. I would however not want to remove it as an isolated change to this page.

Yes, I agree that there are quite some issues. Then also not a lot of urgency to change much. It'd certainly improve the usability and visual appearance of the site, if it was a bit refined and weight of elements reconsidered.
Take the sections h1, why is it a h1? It shouldn't have the same weight or priority as the actual headline "How can we help you?".

The more I look at it, the more of an urge I get to work on this. 😆

Sadly the issue of the visual alignment still remains. If you measure the flexbox or inspect element, it is truly aligned. But it doesn't look aligned. That is because the rightmost column is shorter than the other columns, but they are equal width. As such creating a left-sided balance.

I fixed it with a pl-5. That does the trick and also improves the mobile experience a bit.

Desktop
image

Mobile
image

@MikeMcC399
Copy link
Contributor Author

@brianebeling

Please go ahead as you wish with this PR!

@brianebeling brianebeling merged commit 82fa231 into corona-warn-app:master Nov 29, 2022
brianebeling added a commit that referenced this pull request Nov 29, 2022
@MikeMcC399 MikeMcC399 deleted the fix/faq-center branch November 29, 2022 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants