GCTC-NTGC / gc-digital-talent

GC Digital Talent is the new recruitment platform for digital and tech jobs in the Government of Canada. // Talents numériques du GC est la nouvelle plateforme de recrutement pour les emplois numériques et technologiques au gouvernement du Canada.
https://talent.canada.ca
GNU Affero General Public License v3.0
22 stars 8 forks source link

♻️ Debt/Refactor: Review and fix where Heading component size props have been used improperly #5753

Closed patcon closed 1 year ago

patcon commented 1 year ago

Re-ticketed from https://github.com/GCTC-NTGC/gc-digital-talent/pull/5740#issuecomment-1440357711

♻️ Debt/Refactor

In the above issue, related to <Heading /> component, we stopped defaulting the size prop (for heading styling) to "h2" when it was unspecified. But there were some places where Heading was used that didn't specify size, and so defaulted (likely unintentionally) to "h2", even when the Heading element was something else, e.g. "h3".

In the above PR, we amended all the places this happened to reproduce the original behavior, so that there was no visual diff in the resulting PR.

This ticket is to revisit our use of Heading component and revert if that styling was unintentional.

🕵️ Details

For reference, this is the original Heading component before size's default was removed:

https://github.com/GCTC-NTGC/gc-digital-talent/blob/f387927d33ee024fc8b315b0aa54ca402cde7154/frontend/common/src/components/Heading/Heading.tsx#L15-L41

There are two types of changes this could involve:

  1. Places where data-h2-* props on Heading could be moved into the size prop, and make use of standard Heading styles. e.g.,

    level defaults to h3, size is h2, and manual styling is h4

    https://github.com/GCTC-NTGC/gc-digital-talent/blob/2542c6a9b1bc9989e73eb039cf5b9ade263c42e0/apps/web/src/pages/Pools/BrowsePoolsPage/components/PoolCard/PoolCard.tsx#L108-L116

  2. Places where size="h2" was added (to preserve previous stylings), but is perhaps not intentional. e.g.,

    size h2 added to preserve default h2 styling, but maybe should be simple h6, given the manual override?

    https://github.com/GCTC-NTGC/gc-digital-talent/blob/2542c6a9b1bc9989e73eb039cf5b9ade263c42e0/apps/web/src/pages/PoolCandidates/ViewPoolCandidatePage/components/ApplicationStatusForm/ApplicationStatusForm.tsx#L106-L114

🙋‍♀️ Proposed Solution

Review places where Heading component was used, and look for redundant use of data attributes or mistaken uses of size prop.

To see where size prop was added to mitigate unwanted visual diffs:

🌎 Localization

None.

✅ Acceptance Criteria

A set of assumptions which, when tested, verify that the debt was addressed and expected functionality has not been affected.

tristan-orourke commented 1 year ago

Requires coordination with designers to confirm what size every heading should be.

Maybe we just ask @substrae to go through the site looking for headings of the wrong size?

tristan-orourke commented 1 year ago

@substrae will check headers. Not a dev task.