search

wet-boew / GCWeb

Canada.ca theme - A reference implementation of the Canada.ca Content and Information Architecture Specification, the Canada.ca Content Style Guide and the Canada.ca Design System
https://wet-boew.github.io/GCWeb/
Other
91 stars 130 forks source link

update CSS for high contrast #2413

Closed Christopher-O closed 1 month ago

Christopher-O commented 1 month ago

Added new exception to remove line separator for side by side designs in high contrast mode for firefox as cannot resolve gradient pattern issue (going through text)

Christopher-O commented 1 month ago

@duboisp per your request

Clear and simple explanation of the change/update:

For conjunction (and/or) pattern: Change was to remove the side-by-side line separator in Firefox (but keep the shape and text) when in High contrast mode. The previous pattern had the line separator "cut" through the words (and/or) which could make it slightly illegible.

This does not affect the stacked (default) pattern, as it will still have a visible line to separate the pattern from surrounding content.

For steps pattern: Change was done to make printing documents with the step pattern more easy to see. Previous design had some text cut off at the top or bottom of some steps.

The impact on the sponsored department (CRA) for that change/update:

For conjunction (and/or) pattern: There is minimal impact, as we do not normally use Firefox as a browser, nor does the majority of users use high contrast mode on their devices.

For steps pattern: Minimal impact.

The impact on the public for that change/update:

For conjunction (and/or) pattern: Only those who use high contrast mode with the Firefox browser, and will be a slight improvement on readability of the text

For steps pattern: Improvement to printing document.

Christopher-O commented 1 month ago

See my previous comment.

  • Need review according to our experimental checklist
  • Need a description + impact description.

@duboisp not sure what to put here. These are very minor changes. Nothing substantial that has changed from previous descriptions or review.

Christopher-O commented 1 month ago

Tested and the fix for Conjunction does work. The fix for Steps also works. It seems to add a lot of spacing above the steps which looks a bit weird, but it seems intentional (padding-top: 1em;).

@Christopher-O Could you also add the changes description in the Conjunction meta.md like you did for the Steps and add the history item in the HTML for the Steps like you did for the Conjunction? Thank you!

@Garneauma The media print was added as I noticed that sometimes lines within list items would be cut off close to the border of the page and be unreadable. The padding was added as the CSS numbers created didn't adhere to the break-inside CSS class. But I have tried it on a few other patterns and without the padding, it doesn't seem to do much of a difference (a bit of splitting of the CSS numbers across two pages on the rare occasion. I would approve removing it for now (just the padding-top not the break-inside)

Garneauma commented 1 month ago

@Garneauma The media print was added as I noticed that sometimes lines within list items would be cut off close to the border of the page and be unreadable. The padding was added as the CSS numbers created didn't adhere to the break-inside CSS class. But I have tried it on a few other patterns and without the padding, it doesn't seem to do much of a difference (a bit of splitting of the CSS numbers across two pages on the rare occasion. I would approve removing it for now (just the padding-top not the break-inside)

It's your choice. It was only a comment. :)

Christopher-O commented 1 month ago

@Garneauma The media print was added as I noticed that sometimes lines within list items would be cut off close to the border of the page and be unreadable. The padding was added as the CSS numbers created didn't adhere to the break-inside CSS class. But I have tried it on a few other patterns and without the padding, it doesn't seem to do much of a difference (a bit of splitting of the CSS numbers across two pages on the rare occasion. I would approve removing it for now (just the padding-top not the break-inside)

It's your choice. It was only a comment. :)

But a good one. okay we will leave it in, and comment was added to meta.md and space issue was resolved.

Garneauma commented 1 month ago

@Christopher-O I just realized that the change history is mixed with the implementation plan in conjunction index.html. Could you please separate them? You can do it dynamically by pulling from the metadata like the implementation plan in Steps or you can just do it static for now if you prefer.

Also, please add the extra space in Conjunction meta.md like in the Steps meta.md.

Christopher-O commented 1 month ago

@Christopher-O I just realized that the change history is mixed with the implementation plan in conjunction index.html. Could you please separate them? You can do it dynamically by pulling from the metadata like the implementation plan in Steps or you can just do it static for now if you prefer.

Also, please add the extra space in Conjunction meta.md like in the Steps meta.md.

@Garneauma can you confirm that I completed this correctly?

Garneauma commented 1 month ago

@Christopher-O I have discussed with @duboisp. We will handle the meta.md files on our side when doing the release. So, no more work needed on your part. Thank you for doing the changes!