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

apply h1 thickline globally #2169

Merged
merged 1 commit into from
Feb 13, 2024
Merged

Conversation

jmealing
Copy link
Contributor

@jmealing jmealing commented May 31, 2023

As per DTO email, this is the update for the H1 globally:

  • Removed gc-thickline class and css
  • Modified heading base to apply the thick line styles globally

@jmealing jmealing temporarily deployed to github-ci May 31, 2023 19:48 — with GitHub Actions Inactive
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to check all the proper documentation + working example is also there and up to date.

Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the h1 CSS and create example in this base folder: /sites/h1

Like the breadcrumb folder structure: https://github.com/wet-boew/GCWeb/tree/master/sites/breadcrumbs

  1. Create a single SCSS file for the h1 style
  2. Remove the old h1 style (like you did)
  3. Link that newly SCSS in this main SCSS file, near the other heading style - https://github.com/wet-boew/GCWeb/blob/master/sites/theme.scss#LL35C6-L35C6
  4. Create the dedicated working example (EN and FR) for the h1
  5. Document in those page the version and a code sample
    a. I do think it could be version 1.0.1, a patch change considering there is not change required for the author neither the implementor. We would need to check the public versioning API to be sure. It could be seen as a Minor/Major visual change.
    b. Code sample: I do think we have a variant with the [id=wb-cont] and another one without it. And there is some variant enhance with RDFa + schema.org
  6. Create the index.json-ld file (the name of the componentName need to be same as the folder name.

@delisma delisma temporarily deployed to github-ci July 20, 2023 19:57 — with GitHub Actions Inactive
@delisma delisma temporarily deployed to github-ci July 20, 2023 20:01 — with GitHub Actions Inactive
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to reorganize the documentation according to our latest documentation template prototype.

You can see how the "Change set" and "Iteration" are documented under the technical documentation section.

sites/h1/h1-en.md Outdated Show resolved Hide resolved
@delisma delisma temporarily deployed to github-ci July 21, 2023 19:58 — with GitHub Actions Inactive
@duboisp duboisp added the Query: Project item Part of a github project label Nov 3, 2023
sites/main-page-title/index.json-ld Outdated Show resolved Hide resolved
_data/sites.json Outdated Show resolved Hide resolved
components/baseline/_base.scss Outdated Show resolved Hide resolved
sites/main-page-title/includes/main-page-title.html Outdated Show resolved Hide resolved
sites/main-page-title/includes/main-page-title.html Outdated Show resolved Hide resolved
sites/main-page-title/includes/main-page-title.html Outdated Show resolved Hide resolved
sites/main-page-title/includes/main-page-title.html Outdated Show resolved Hide resolved
sites/main-page-title/_main-page-title.scss Outdated Show resolved Hide resolved
sites/main-page-title/main-page-title-en.md Outdated Show resolved Hide resolved
sites/main-page-title/index.json-ld Show resolved Hide resolved
@delisma
Copy link

delisma commented Jan 22, 2024

@duboisp Can you remove me and Eric as reviewers? Can you remove Jennifer as an assignee?

Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should clean up, which can be done on or immediately after, remove any use of "gc-thickline" in our pages.

Do we have a note about that somewhere saying that CSS class is useless because it is now the default for h1?

sites/main-page-title/main-page-title.json Outdated Show resolved Hide resolved
sites/main-page-title/main-page-title-en.md Outdated Show resolved Hide resolved
sites/main-page-title/main-page-title-fr.md Outdated Show resolved Hide resolved
sites/_variables.scss Outdated Show resolved Hide resolved
sites/_variables.scss Outdated Show resolved Hide resolved
_data/sites.json Outdated Show resolved Hide resolved
@GormFrank
Copy link
Collaborator

Pre-approved for this week provided @delisma makes the changes requested by @duboisp, which are small changes that don't affect the core purpose of this PR.

This commit adds the main page title component, which includes the SCSS file, HTML template, JSON data, and Markdown documentation for both English and French versions. The component provides a default H1 style with a short bold red underline for use in Canada.ca websites. It also includes guidance and code samples for implementing the component with and without RDFa + schema.org markup.
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review and tested. It does work as expected.

Once this PR is merged, the following 2 actions is required immediatly after or they can be included with the release PR.

  • Add the technical note about the obsoleted gc-thickline in the index.json-ld
  • Regenerate the GCWeb sites assets (ex: _data/sites.json ...) to keep GCWeb web site functional.

@duboisp
Copy link
Member

duboisp commented Feb 12, 2024

Pre-approved upon quick review of the unresolved conversation by @duboisp

@duboisp duboisp merged commit f262e39 into wet-boew:master Feb 13, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Query: Project item Part of a github project
Projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

5 participants