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

adjust and remove heading css #139

Merged
merged 5 commits into from
Oct 24, 2023
Merged

adjust and remove heading css #139

merged 5 commits into from
Oct 24, 2023

Conversation

jatindersingh93
Copy link
Contributor

@jatindersingh93 jatindersingh93 commented Oct 20, 2023

Removed, adjust and reuse heading tags.
This should fix reported formatting issue to login/logout page and other unreported inconsistency.

Description

We have to make sure all headings are being displayed properly in terms of size and location on screen.
This should fix login/logout and other inconsistency caused by adjusting heading tags.

Types of changes

Checklist

  • I have read the CONTRIBUTING doc
  • I have checked that unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@github-actions
Copy link

Coverage Report (Application)

Totals Coverage
Statements: 75% ( 51 / 68 )
Methods: 62.5% ( 5 / 8 )
Lines: 82.61% ( 38 / 46 )
Branches: 57.14% ( 8 / 14 )

@github-actions
Copy link

Coverage Report (Frontend)

Totals Coverage
Statements: 36% ( 604 / 1678 )
Methods: 35.58% ( 132 / 371 )
Lines: 43.21% ( 423 / 979 )
Branches: 14.94% ( 49 / 328 )

@jatindersingh93 jatindersingh93 marked this pull request as ready for review October 20, 2023 18:39
@TimCsaky
Copy link
Contributor

TimCsaky commented Oct 20, 2023

the changes look good. those pages were pretty hidden!

more of a visual/css issue for me but, i wonder if these h1 headings need to be bold, others h1's are font-weight 400.
or maybe they all should be.
maybe the one in the sidebar should not be h1.
image

this heading as well is maybe inconsistent.
image
maybe we ask @tpantella

@tpantella
Copy link

tpantella commented Oct 20, 2023

maybe we ask @tpantella

Hi! I think "Select a bucket" looks correct, but whatever more closely matches h1 on gov.bc.ca, such as this example:
https://www2.gov.bc.ca/gov/content/health/health-drug-coverage/pharmacare-for-bc-residents/drug-review-process-results/drug-review-decisions
"Drug review decisions" is the correct styling and weight. We should try to mimic this with our H1s. You can jump around on gov.bc.ca to see examples of H2 and H3 as well if that is of interest.

On an object list, the "Files" heading seems to be the wrong weight, it should be consistent with the above.
On a full file details page, the "File details" heading seems to be the wrong weight, it should match the above.

On the side pane heading, which is what Tim was actually asking about, I believe it's ok to have H1s in the side pane - it's ok to use multiple H1s, the side pane is acting as a sort of framed page, and it's a separate and overarching topic that earns an H1. I could also see a case for it being H2, which would then force any child headings to be H3, and children of children to be H4. My preference is H1 since it must draw attention to itself and becomes the new "main topic" being observed on the page. If a developer has a strong preference and makes a case for it, or if H2 for side pane headers and cascading all other sizes down one is easier and looks good, I am open to that. This comes down more to what looks good, since there is a style justification for either approach.

Design System guidance on weights is here if this helps:
https://developer.gov.bc.ca/Design-System/Typography

I think specified weights rather than specified bold styling is interpreted differently by screen reader software, where bold can invoke a shouty voice or emphasis, and I think we should also try to avoid variants of headings in bold and non-bold at the same H-level. (Unless there ends up being a need for it! Which we may already have. Like a filename or bucket name for example. Those would be okay. I'm mainly talking about actual ordinary headings.)

tl;dr - I think we need to make all H1s heavy in weight but not explicitly stated as "bold", I think some need fixing, I think the side pane heading could be H1 or H2 depending on what looks best, some adjustments may be needed on children headings if it goes to H2

@jujaga jujaga merged commit 4f22385 into master Oct 24, 2023
14 checks passed
@jujaga jujaga deleted the sc3004 branch October 24, 2023 16:54
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.

4 participants