Skip to content

Mitigate the use of inline style as a CSS class#36

Merged
duboisp merged 1 commit intoServiceCanada:lts-updatefrom
polmih:wet-657
Mar 31, 2026
Merged

Mitigate the use of inline style as a CSS class#36
duboisp merged 1 commit intoServiceCanada:lts-updatefrom
polmih:wet-657

Conversation

@polmih
Copy link
Copy Markdown
Collaborator

@polmih polmih commented Mar 30, 2026

WET-657 related

Copy link
Copy Markdown

@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.

See my comment inline.

Comment thread Gruntfile.js
Comment thread examples/oag-inline-style-to-classes.html Outdated
Comment thread examples/oag-inline-style-to-classes.html Outdated
Comment thread examples/oag-inline-style-to-classes.html
Comment thread _layouts/core.html Outdated
Comment thread assets/oag-font-css-fix.css Outdated
Copy link
Copy Markdown

@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.

Reviewed and tested the build script update, it does generate the css file as expected.

I did a functional test via my github pages and the added css class are not applied to the page, there is an issue with the path of the css file.

A few additional content change are requires.

Comment thread assets/oag-font-css-fix.css Outdated
Comment thread examples/oag-inline-style-to-classes.html
Comment thread examples/oag-inline-style-to-classes.html
@polmih
Copy link
Copy Markdown
Collaborator Author

polmih commented Mar 30, 2026

Reviewed and tested the build script update, it does generate the css file as expected.

I did a functional test via my github pages and the added css class are not applied to the page, there is an issue with the path of the css file.

A few additional content change are requires.

Path is now adjusted, it's working now on gihub pages: https://polmih.github.io/oag-template/examples/oag-inline-style-to-classes.html

@polmih polmih requested a review from duboisp March 30, 2026 20:03
Copy link
Copy Markdown

@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 can see the stylesheet to be added now and the warning message seem to be descriptive enough.

But I can't find the oag-purple-trillium css class. It was probably not released yet. For our immediate need, can you create that style as a temporary CSS for now.

Thanks

Comment thread examples/oag-inline-style-to-classes.html Outdated
Copy link
Copy Markdown

@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.

One small content change.

  • Complement the description inside the danger notice to match and have it aligned with the description in the CSS file.

Other than that, we should be good after to initiate a release based on the latest tag v1.3.1. That may require to adjust the base branch for this PR and then cloning this PR. I will help with that technicality.

Comment thread examples/oag-inline-style-to-classes.html Outdated
@polmih polmih requested a review from duboisp March 31, 2026 12:54
@polmih polmih changed the base branch from main to lts-update March 31, 2026 14:11
Copy link
Copy Markdown

@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.

Look good,

I reviewed and tested and it work as expected.

Description: Mitigate the use of inline style via a temporary strategy because inline style applied through the font element are going to lost their effect after our upcoming content management system upgrade.
Impact on the sponsored department: The removal of those temporary style will be added into their maintenance plan and they will need to replace them and fix their pages by using a long term solution.
Impact on the public: None, content will be rendered as it is currently.

@duboisp duboisp merged commit 90eb87e into ServiceCanada:lts-update Mar 31, 2026
1 check passed
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.

2 participants