Skip to content

Commit eed7f79

Browse files
shakyShaneshakyShane
authored
ntp: Fixed animation jitter of customizer drawer (#1386)
* Ship review feedback * give a theme context to customizer button
1 parent 9635bd7 commit eed7f79

18 files changed

+143
-51
lines changed

‎special-pages/pages/new-tab/app/components/App.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
import { Fragment, h } from 'preact';
22
import cn from 'classnames';
33
import styles from './App.module.css';
4-
import { useCustomizerDrawerSettings, usePlatformName } from '../settings.provider.js';
4+
import { useCustomizerDrawerSettings, useCustomizerKind, usePlatformName } from '../settings.provider.js';
55
import { WidgetList } from '../widget-list/WidgetList.js';
66
import { useGlobalDropzone } from '../dropzone.js';
7-
import { Customizer, CustomizerButton, CustomizerMenuPositionedFixed, useContextMenu } from '../customizer/components/Customizer.js';
7+
import {
8+
CustomizerMenu,
9+
CustomizerButton,
10+
CustomizerMenuPositionedFixed,
11+
useContextMenu,
12+
} from '../customizer/components/CustomizerMenu.js';
813
import { useDrawer, useDrawerControls } from './Drawer.js';
914
import { CustomizerDrawer } from '../customizer/components/CustomizerDrawer.js';
1015
import { BackgroundConsumer } from './BackgroundProvider.js';
@@ -23,7 +28,7 @@ export function App() {
2328
const platformName = usePlatformName();
2429
const customizerDrawer = useCustomizerDrawerSettings();
2530

26-
const customizerKind = customizerDrawer.state === 'enabled' ? 'drawer' : 'menu';
31+
const customizerKind = useCustomizerKind();
2732

2833
useGlobalDropzone();
2934
useContextMenu();
@@ -41,6 +46,7 @@ export function App() {
4146
} = useDrawer(customizerDrawer.autoOpen ? 'visible' : 'hidden');
4247

4348
const tabIndex = useComputed(() => (hidden.value ? -1 : 0));
49+
const isOpen = useComputed(() => hidden.value === false);
4450
const { toggle } = useDrawerControls();
4551
const { main, browser } = useContext(CustomizerThemesContext);
4652

@@ -54,19 +60,22 @@ export function App() {
5460
<WidgetList />
5561
</div>
5662
</div>
63+
</main>
64+
<div data-theme={main}>
5765
<CustomizerMenuPositionedFixed>
58-
{customizerKind === 'menu' && <Customizer />}
66+
{customizerKind === 'menu' && <CustomizerMenu />}
5967
{customizerKind === 'drawer' && (
6068
<CustomizerButton
6169
buttonId={buttonId}
6270
menuId={drawerId}
6371
toggleMenu={toggle}
6472
buttonRef={buttonRef}
65-
isOpen={false}
73+
isOpen={isOpen}
74+
kind={'drawer'}
6675
/>
6776
)}
6877
</CustomizerMenuPositionedFixed>
69-
</main>
78+
</div>
7079
{customizerKind === 'drawer' && (
7180
<aside
7281
class={cn(styles.aside, styles.asideLayout, styles.asideScroller)}

‎special-pages/pages/new-tab/app/components/App.module.css

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,14 @@ body[data-animate-background="true"] {
5252
color: var(--ntp-text-normal);
5353
}
5454

55+
56+
5557
.mainLayout {
56-
padding-right: 0;
57-
transition: padding-right .3s;
58+
will-change: transform;
59+
transition: transform .3s;
60+
5861
[data-drawer-visibility='visible'] & {
59-
padding-right: var(--ntp-combined-width);
62+
transform: translateX(calc(0px - var(--ntp-combined-width) / 2));
6063
}
6164
}
6265

@@ -110,6 +113,7 @@ body[data-animate-background="true"] {
110113
right: 0;
111114
top: 0;
112115
z-index: 1;
116+
will-change: transform;
113117
transform: translateX(100%);
114118
transition: transform .3s;
115119

‎special-pages/pages/new-tab/app/customizer/components/BackgroundSection.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ function DefaultPanel({ checked, onClick }) {
9393
aria-labelledby={id}
9494
role="radio"
9595
onClick={onClick}
96+
tabindex={checked ? -1 : 0}
9697
>
9798
{checked && <CircleCheck />}
9899
</button>
@@ -117,6 +118,7 @@ function ColorPanel(props) {
117118
data-color-mode={props.color.colorScheme}
118119
onClick={props.onClick}
119120
aria-checked={props.checked}
121+
tabindex={props.checked ? -1 : 0}
120122
aria-labelledby={id}
121123
role="radio"
122124
style={{ background: props.color.hex }}
@@ -144,6 +146,7 @@ function GradientPanel(props) {
144146
class={cn(styles.bgPanel, styles.dynamicIconColor)}
145147
data-color-mode={props.gradient.colorScheme}
146148
aria-checked={props.checked}
149+
tabindex={props.checked ? -1 : 0}
147150
aria-labelledby={id}
148151
style={{
149152
background: `url(${props.gradient.path})`,

‎special-pages/pages/new-tab/app/customizer/components/BrowserThemeSection.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export function BrowserThemeSection(props) {
2626
role="radio"
2727
type="button"
2828
aria-checked={current.value === 'light'}
29-
tabindex={0}
29+
tabindex={current.value === 'light' ? -1 : 0}
3030
onClick={() => props.setTheme({ theme: 'light' })}
3131
>
3232
<span class="sr-only">{t('customizer_browser_theme_label', { type: 'light' })}</span>
@@ -39,7 +39,7 @@ export function BrowserThemeSection(props) {
3939
role="radio"
4040
type="button"
4141
aria-checked={current.value === 'dark'}
42-
tabindex={0}
42+
tabindex={current.value === 'dark' ? -1 : 0}
4343
onClick={() => props.setTheme({ theme: 'dark' })}
4444
>
4545
<span class="sr-only">{t('customizer_browser_theme_label', { type: 'dark' })}</span>
@@ -52,7 +52,7 @@ export function BrowserThemeSection(props) {
5252
role="radio"
5353
type="button"
5454
aria-checked={current.value === 'system'}
55-
tabindex={0}
55+
tabindex={current.value === 'system' ? -1 : 0}
5656
onClick={() => props.setTheme({ theme: 'system' })}
5757
>
5858
<span class="sr-only">{t('customizer_browser_theme_label', { type: 'system' })}</span>

‎special-pages/pages/new-tab/app/customizer/components/Customizer.examples.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { h } from 'preact';
22
import { noop } from '../../utils.js';
3-
import { CustomizerButton } from './Customizer.js';
3+
import { CustomizerButton } from './CustomizerMenu.js';
44
import { EmbeddedVisibilityMenu, VisibilityMenu } from './VisibilityMenu.js';
55
import { BackgroundSection } from './BackgroundSection.js';
66
import { ColorSelection } from './ColorSelection.js';
@@ -66,7 +66,7 @@ export const customizerExamples = {
6666
'customizer-menu': {
6767
factory: () => (
6868
<MaxContent>
69-
<CustomizerButton isOpen={true} />
69+
<CustomizerButton isOpen={true} kind="menu" />
7070
<br />
7171
<VisibilityMenu
7272
rows={[

‎special-pages/pages/new-tab/app/customizer/components/Customizer.module.css

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
}
44

55
.lowerRightFixed {
6-
position: fixed;
6+
position: absolute;
77
bottom: 1rem;
88
right: 1rem;
99
}
@@ -56,6 +56,16 @@
5656
border-color: var(--color-white-at-50);
5757
}
5858
}
59+
60+
&[data-kind="drawer"][aria-expanded="true"] {
61+
visibility: hidden;
62+
}
63+
64+
@media screen and (max-width: 800px) {
65+
span {
66+
display: none;
67+
}
68+
}
5969
}
6070

6171

‎special-pages/pages/new-tab/app/customizer/components/CustomizerDrawerInner.module.css

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@
6363
opacity: .8;
6464
}
6565
&:focus-visible {
66-
outline: 1px solid var(--ntp-focus-outline-color)
66+
box-shadow: var(--focus-ring);
6767
}
6868
}
6969
.section {

‎special-pages/pages/new-tab/app/customizer/components/Customizer.js renamed to ‎special-pages/pages/new-tab/app/customizer/components/CustomizerMenu.js

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { VisibilityMenu, VisibilityMenuPopover } from './VisibilityMenu.js';
1313
/**
1414
* Represents the NTP customizer. For now it's just the ability to toggle sections.
1515
*/
16-
export function Customizer() {
16+
export function CustomizerMenu() {
1717
const { setIsOpen, buttonRef, dropdownRef, isOpen } = useDropdown();
1818
const [rowData, setRowData] = useState(/** @type {VisibilityRowData[]} */ ([]));
1919

@@ -32,9 +32,9 @@ export function Customizer() {
3232
function handler() {
3333
setRowData(getItems());
3434
}
35-
window.addEventListener(Customizer.UPDATE_EVENT, handler);
35+
window.addEventListener(CustomizerMenu.UPDATE_EVENT, handler);
3636
return () => {
37-
window.removeEventListener(Customizer.UPDATE_EVENT, handler);
37+
window.removeEventListener(CustomizerMenu.UPDATE_EVENT, handler);
3838
};
3939
}, [isOpen]);
4040

@@ -43,7 +43,14 @@ export function Customizer() {
4343

4444
return (
4545
<div class={styles.root} ref={dropdownRef}>
46-
<CustomizerButton buttonId={BUTTON_ID} menuId={MENU_ID} toggleMenu={toggleMenu} buttonRef={buttonRef} isOpen={isOpen} />
46+
<CustomizerButton
47+
buttonId={BUTTON_ID}
48+
menuId={MENU_ID}
49+
toggleMenu={toggleMenu}
50+
buttonRef={buttonRef}
51+
isOpen={isOpen}
52+
kind={'menu'}
53+
/>
4754
<div id={MENU_ID} class={cn(styles.dropdownMenu, { [styles.show]: isOpen })} aria-labelledby={BUTTON_ID}>
4855
<VisibilityMenuPopover>
4956
<VisibilityMenu rows={rowData} />
@@ -53,8 +60,8 @@ export function Customizer() {
5360
);
5461
}
5562

56-
Customizer.OPEN_EVENT = 'ntp-customizer-open';
57-
Customizer.UPDATE_EVENT = 'ntp-customizer-update';
63+
CustomizerMenu.OPEN_EVENT = 'ntp-customizer-open';
64+
CustomizerMenu.UPDATE_EVENT = 'ntp-customizer-update';
5865

5966
export function getItems() {
6067
/** @type {VisibilityRowData[]} */
@@ -64,7 +71,7 @@ export function getItems() {
6471
next.push(incoming);
6572
},
6673
};
67-
const event = new CustomEvent(Customizer.OPEN_EVENT, { detail });
74+
const event = new CustomEvent(CustomizerMenu.OPEN_EVENT, { detail });
6875
window.dispatchEvent(event);
6976
next.sort((a, b) => a.index - b.index);
7077
return next;
@@ -102,20 +109,22 @@ export function useContextMenu() {
102109
* @param {object} props
103110
* @param {string} [props.menuId]
104111
* @param {string} [props.buttonId]
105-
* @param {boolean} props.isOpen
112+
* @param {import("@preact/signals").Signal<boolean>|boolean} props.isOpen
106113
* @param {() => void} [props.toggleMenu]
107114
* @param {import("preact").Ref<HTMLButtonElement>} [props.buttonRef]
115+
* @param {"menu" | "drawer"} props.kind
108116
*/
109-
export function CustomizerButton({ menuId, buttonId, isOpen, toggleMenu, buttonRef }) {
117+
export function CustomizerButton({ menuId, buttonId, isOpen, toggleMenu, buttonRef, kind }) {
110118
const { t } = useTypedTranslation();
111119
return (
112120
<button
113121
ref={buttonRef}
114-
className={styles.customizeButton}
122+
class={styles.customizeButton}
115123
onClick={toggleMenu}
116124
aria-haspopup="true"
117125
aria-expanded={isOpen}
118126
aria-controls={menuId}
127+
data-kind={kind}
119128
id={buttonId}
120129
>
121130
<CustomizeIcon />
@@ -210,11 +219,11 @@ export function useCustomizer({ title, id, icon, toggle, visibility, index }) {
210219
const handler = (/** @type {CustomEvent<any>} */ e) => {
211220
e.detail.register({ title, id, icon, toggle, visibility, index });
212221
};
213-
window.addEventListener(Customizer.OPEN_EVENT, handler);
214-
return () => window.removeEventListener(Customizer.OPEN_EVENT, handler);
222+
window.addEventListener(CustomizerMenu.OPEN_EVENT, handler);
223+
return () => window.removeEventListener(CustomizerMenu.OPEN_EVENT, handler);
215224
}, [title, id, icon, toggle, visibility, index]);
216225

217226
useEffect(() => {
218-
window.dispatchEvent(new Event(Customizer.UPDATE_EVENT));
227+
window.dispatchEvent(new Event(CustomizerMenu.UPDATE_EVENT));
219228
}, [visibility]);
220229
}

‎special-pages/pages/new-tab/app/customizer/components/VisibilityMenu.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { CustomizerThemesContext } from '../CustomizerProvider.js';
1111

1212
/**
1313
* @import { Widgets, WidgetConfigItem } from '../../../types/new-tab.js'
14-
* @import { VisibilityRowData, VisibilityRowState } from './Customizer.js'
14+
* @import { VisibilityRowData, VisibilityRowState } from './CustomizerMenu.js'
1515
*/
1616

1717
/**

‎special-pages/pages/new-tab/app/customizer/components/VisibilityMenu.module.css

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
}
2525

2626
.embedded {
27-
font-size: var(--small-label-font-size);
27+
font-size: var(--body-font-size);
2828
gap: 12px;
2929
}
3030

@@ -34,22 +34,21 @@
3434
gap: 10px;
3535
white-space: nowrap;
3636
height: calc(28 * var(--px-in-rem));
37-
38-
> * {
39-
min-width: 0;
40-
}
41-
42-
label {
43-
margin-left: auto;
44-
}
4537
}
4638

4739
.menuItemLabelEmbedded {
4840
white-space: normal;
4941
gap: 6px;
5042
height: auto;
51-
> * {
52-
min-width: auto;
43+
44+
/* push the input over to the RHS */
45+
*:last-child {
46+
margin-left: auto;
47+
}
48+
49+
/* Apply a theme-specific focus ring on the switch **/
50+
input:focus-visible + * {
51+
box-shadow: var(--focus-ring)
5352
}
5453
}
5554

0 commit comments

Comments
 (0)