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.

Sign up for GitHub

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

fix: Don't use dark mode for print media #1204

Merged
merged 2 commits into from
Jun 15, 2023
Merged

fix: Don't use dark mode for print media #1204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a4a79f
fix: Don't use dark mode for print media
gethinwebster Jun 9, 2023
07d417d
Document JS media print exclusion
gethinwebster Jun 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/internal/hooks/use-visual-mode/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import { useMutationObserver } from '../use-mutation-observer';
import { isDevelopment } from '../../is-development';
import { warnOnce } from '../../logging';

// Note that this hook doesn't take into consideration @media print (unlike the dark mode CSS),
Copy link
Member

Choose a reason for hiding this comment

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

What was wrong with using !window?.matchMedia?.('print').matches?

Copy link
Member Author

Choose a reason for hiding this comment

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

window.matchMedia('print').matches changes briefly to true and then immediately back to false when opening the print dialog, which doesn't give enough time for React to re-render.

window.addEventListener('beforeprint') (and afterprint) works better in some browsers but not consistently.

In general, the amount of & complexity of code that looked like it would be required for this seemed to outweigh the benefits of the usecase (it impacts CodeEditor and dropdown elements rendered in portals)

Copy link
Member

Choose a reason for hiding this comment

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

I see, but what is the scope of the fix now? The dark-mode-only mixin is only used for icons in the Code editor component, and I am no able to find any difference by manually testing in Chrome (nor in the Code editor component pages nor elsewhere).

Copy link
Member Author

Choose a reason for hiding this comment

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

All CSS-based dark mode styling should be removed when printing. However, I think the PR deployment isn't showing this up because the supporting change (cloudscape-design/theming-core@5febd2e) has not yet made it through our internal pipelines

// due to challenges with cross-browser implementations of media/print state change listeners.
// This means that components using this hook will render in dark mode even when printing.
export function useCurrentMode(elementRef: React.RefObject<HTMLElement>) {
const [value, setValue] = useState<'light' | 'dark'>('light');
useMutationObserver(elementRef, node => {
Expand Down
10 changes: 6 additions & 4 deletions src/internal/styles/utils/theming.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
SPDX-License-Identifier: Apache-2.0
*/
@mixin dark-mode-only($selector: '') {
/* stylelint-disable selector-combinator-disallowed-list, selector-class-pattern */
:global(#{$selector}.awsui-polaris-dark-mode) &,
:global(#{$selector}.awsui-dark-mode) & {
@content;
@media not print {
/* stylelint-disable selector-combinator-disallowed-list, selector-class-pattern */
:global(#{$selector}.awsui-polaris-dark-mode) &,
:global(#{$selector}.awsui-dark-mode) & {
@content;
}
}
}
2 changes: 1 addition & 1 deletion style-dictionary/utils/modes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export const createColorMode = (darkSelector: string) => ({
id: 'color',
states: {
light: { default: true },
dark: { selector: darkSelector },
dark: { selector: darkSelector, media: 'not print' },
},
});

Expand Down