Feature: Predefined alerts with customizable colors and headings - #1602
Feature: Predefined alerts with customizable colors and headings #1602pdmosses wants to merge 5 commits into
Conversation
See docs/ui-components/alerts.md for how to use alerts.
|
Oh, this is super exciting work @pdmosses! I can aim to give a more in-depth review ... "as soon as possible" (maybe over the next few days? have some pending committee work 😅 ). As a big picture question: are you envisioning having this side-by-side with callouts, and having users decide which version they prefer? Or eventually pushing users to use alerts over callouts (or callouts over alerts)? I wonder if there's a way that we can reduce the maintenance burden of running two features like this in parallel (especially when there are accessibility considerations that limit our color palette choice). |
|
@mattxwang thanks for your initial response. I'm looking forward to your review!
I think it will be difficult to improve the accessibility of callouts – at least when they are configured using the colors defined by the theme. This is partly because all callouts use only the Alerts can (and currently do) use different shades for different colors, and don't have colored backgrounds. Moreover, the shades for the light and dark color schemes are independent. I hope their accessibility will be significantly better than callouts. And alerts are implemented entirely in SCSS, which might be easier to maintain (and understand!) than the Liquid+SCSS code needed to implement configuration-dependent callouts. I suggest to continue to support callouts, but without maintenance obligations. The docs should perhaps deprecate their configuration with some of the theme colors in the theme color schemes. |
|
Thank you for this PR, it really helped a lot! I only adjusted styling a bit to match old callouts since I liked them a bit more with a background |
|
@vladysor I'm glad you found the PR useful! Thanks for the feedback.
My main motivation for changing the styling from the old callouts (apart from simplifying the CSS) was to match GitHub Alerts. However, background colors can reduce contrast, which might be an accessibility concern for alert colors that are predefined by the theme. |
#1748) This PR slightly reimagines callouts to fix longstanding accessibility issues (on the light theme). By removing the `background-color` from them, body text for the callout now will always have sufficient background contrast (as the background is the same as the overall theme). This includes cases where users use other styles that modify text color, such as links or `code` snippets. As opacity is now no longer relevant, that option has been removed. Users who have it enabled will see no difference, so this is not breaking in *that* sense. Separately, I've changed "loud" to not use any of the lighter colors, which is necessary to achieve proper contrast in light mode. I'll revisit dark mode later (see below). -- Note that this PR does not fix these *other* issues with callouts: - in the dark theme, the provided colors do not contrast sufficiently with the black background, which particularly affects callout titles. I will likely fix this in a dark theme pass, rather than in this PR. (I will most likely pick a different color palette for the dark theme, but will need to ponder) - separately, callouts can "inherit" other issues with the dark theme, e.g. the link color not contrasting sufficiently. I'll resolve this separately as well. -- I'll include a note in the migration docs on how to revert this change, as I understand that some users like having the background color. But, my hope is in the long-term to instead merge in @pdmosses work in #1602 and then keep this callout feature in maintenance mode.
just-the-docs#1748) This PR slightly reimagines callouts to fix longstanding accessibility issues (on the light theme). By removing the `background-color` from them, body text for the callout now will always have sufficient background contrast (as the background is the same as the overall theme). This includes cases where users use other styles that modify text color, such as links or `code` snippets. As opacity is now no longer relevant, that option has been removed. Users who have it enabled will see no difference, so this is not breaking in *that* sense. Separately, I've changed "loud" to not use any of the lighter colors, which is necessary to achieve proper contrast in light mode. I'll revisit dark mode later (see below). -- Note that this PR does not fix these *other* issues with callouts: - in the dark theme, the provided colors do not contrast sufficiently with the black background, which particularly affects callout titles. I will likely fix this in a dark theme pass, rather than in this PR. (I will most likely pick a different color palette for the dark theme, but will need to ponder) - separately, callouts can "inherit" other issues with the dark theme, e.g. the link color not contrasting sufficiently. I'll resolve this separately as well. -- I'll include a note in the migration docs on how to revert this change, as I understand that some users like having the background color. But, my hope is in the long-term to instead merge in @pdmosses work in just-the-docs#1602 and then keep this callout feature in maintenance mode.
just-the-docs#1748) This PR slightly reimagines callouts to fix longstanding accessibility issues (on the light theme). By removing the `background-color` from them, body text for the callout now will always have sufficient background contrast (as the background is the same as the overall theme). This includes cases where users use other styles that modify text color, such as links or `code` snippets. As opacity is now no longer relevant, that option has been removed. Users who have it enabled will see no difference, so this is not breaking in *that* sense. Separately, I've changed "loud" to not use any of the lighter colors, which is necessary to achieve proper contrast in light mode. I'll revisit dark mode later (see below). -- Note that this PR does not fix these *other* issues with callouts: - in the dark theme, the provided colors do not contrast sufficiently with the black background, which particularly affects callout titles. I will likely fix this in a dark theme pass, rather than in this PR. (I will most likely pick a different color palette for the dark theme, but will need to ponder) - separately, callouts can "inherit" other issues with the dark theme, e.g. the link color not contrasting sufficiently. I'll resolve this separately as well. -- I'll include a note in the migration docs on how to revert this change, as I understand that some users like having the background color. But, my hope is in the long-term to instead merge in @pdmosses work in just-the-docs#1602 and then keep this callout feature in maintenance mode.
This PR adds alerts as an alternative to callouts. See also #1600 (comment).
The kinds and rendering of the predefined alerts are similar to the alerts supported on GitHub. However, the colors and headings can be customized, and further kinds of alerts can be added.
See the new docs page for Alerts in the preview for further details.