Skip to content

fix: Implement TopNavigation breakpoints in CSS #3371

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions src/internal/components/menu-dropdown/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ export const ButtonTrigger = React.forwardRef(
expanded,
children,
onClick,
// Used by TopNavigation to apply styles.
className,
}: ButtonTriggerProps,
ref: React.Ref<any>
) => {
Expand All @@ -40,7 +42,7 @@ export const ButtonTrigger = React.forwardRef(
<button
ref={ref}
type="button"
className={clsx(styles.button, styles[`offset-right-${offsetRight}`], testUtilsClass, {
className={clsx(styles.button, styles[`offset-right-${offsetRight}`], testUtilsClass, className, {
[styles.expanded]: expanded,
})}
aria-label={ariaLabel}
Expand All @@ -49,7 +51,7 @@ export const ButtonTrigger = React.forwardRef(
disabled={disabled}
onClick={event => {
event.preventDefault();
onClick && onClick();
onClick?.();
}}
>
{hasIcon && (
Expand Down Expand Up @@ -79,6 +81,7 @@ const MenuDropdown = ({
badge,
offsetRight,
children,
className,
...props
}: MenuDropdownProps) => {
const baseProps = getBaseProps(props);
Expand All @@ -94,6 +97,7 @@ const MenuDropdown = ({
return (
<ButtonTrigger
testUtilsClass={testUtilsClass}
className={className}
ref={triggerRef}
disabled={disabled}
expanded={isOpen}
Expand Down
5 changes: 4 additions & 1 deletion src/internal/components/menu-dropdown/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { IconProps } from '../../../icon/interfaces';

export interface ButtonTriggerProps {
testUtilsClass?: string;
className?: string;
iconName?: IconProps.Name;
iconUrl?: string;
iconAlt?: string;
Expand All @@ -19,12 +20,14 @@ export interface ButtonTriggerProps {
expanded?: boolean;
}

export interface MenuDropdownProps extends InternalButtonDropdownProps {
export interface MenuDropdownProps extends Omit<InternalButtonDropdownProps, 'items'> {
items: InternalButtonDropdownProps['items'];
iconName?: IconProps.Name;
iconUrl?: string;
iconAlt?: string;
iconSvg?: React.ReactNode;
badge?: boolean;
description?: string;
offsetRight?: 'l' | 'xxl';
className?: string;
}
38 changes: 37 additions & 1 deletion src/internal/styles/foundation/breakpoints.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
SPDX-License-Identifier: Apache-2.0
*/

// Breakpoints
// Media query breakpoints
$breakpoint-xxx-small: 0;
$breakpoint-xx-small: 576px;
$breakpoint-x-small: 688px;
Expand All @@ -16,6 +16,18 @@ $breakpoint-xx-large: 2540px;
$_smallest_breakpoint: $breakpoint-xxx-small;
$_largest_breakpoint: $breakpoint-x-large;

// Container breakpoints, matching the Grid component
$container-breakpoint-default: -1;
$container-breakpoint-xxs: 465px;
$container-breakpoint-xs: 688px;
$container-breakpoint-s: 912px;
$container-breakpoint-m: 1120px;
$container-breakpoint-l: 1320px;
$container-breakpoint-xl: 1840px;

$_container_smallest_breakpoint: $container-breakpoint-default;
$_container_largest_breakpoint: $container-breakpoint-xl;

// Media of at least the minimum breakpoint width. No query for the smallest breakpoint.
// Makes the @content apply to the wider than given breakpoint.
@mixin media-breakpoint-up($breakpoint) {
Expand All @@ -39,3 +51,27 @@ $_largest_breakpoint: $breakpoint-x-large;
@content;
}
}

// Container query for widths greater than the given breakpoint.
Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍

// Matches the behavior of getMatchingBreakpoint in breakpoints.ts
@mixin container-breakpoint-up($breakpoint) {
@if $breakpoint != $_container_smallest_breakpoint {
@container (min-width: #{$breakpoint + 1px}) {
@content;
}
} @else {
@content;
}
}

// Container query for widths less than or equal to the given breakpoint.
// Matches the behavior of matchBreakpointMapping in breakpoints.ts
@mixin container-breakpoint-down($breakpoint) {
@if $breakpoint != $_container_largest_breakpoint {
@container (max-width: #{$breakpoint}) {
@content;
}
} @else {
@content;
}
}
4 changes: 2 additions & 2 deletions src/test-utils/dom/top-navigation/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,10 @@ export class TopNavigationMenuDropdownWrapper extends ButtonDropdownWrapper {
}

findTitle(): ElementWrapper | null {
return this.findByClassName(buttonDropdownStyles.title);
return createWrapper().findComponent(`.${buttonDropdownStyles.title}`, ElementWrapper);
Copy link
Member

Choose a reason for hiding this comment

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

What is the reason of this change?

Copy link
Author

Choose a reason for hiding this comment

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

Without this change, the test

TopNavigation Utility part › Menu dropdown › has a title in the dropdown

fails to find the title element, due to the menu dropdown having expandToViewport.

}

findDescription(): ElementWrapper | null {
return this.findByClassName(buttonDropdownStyles.description);
return createWrapper().findComponent(`.${buttonDropdownStyles.description}`, ElementWrapper);
}
}
96 changes: 29 additions & 67 deletions src/top-navigation/internal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,13 @@ export default function InternalTopNavigation({
}: InternalTopNavigationProps) {
checkSafeUrl('TopNavigation', identity.href);
const baseProps = getBaseProps(restProps);
const { mainRef, virtualRef, breakpoint, responsiveState, isSearchExpanded, onSearchUtilityClick } = useTopNavigation(
{ identity, search, utilities }
);
const { mainRef, virtualRef, responsiveState, isSearchExpanded, onSearchUtilityClick } = useTopNavigation({
identity,
search,
utilities,
});
const [overflowMenuOpen, setOverflowMenuOpen] = useState(false);
const overflowMenuTriggerRef = useRef<HTMLButtonElement>(null);
const isNarrowViewport = breakpoint === 'default';
const isMediumViewport = breakpoint === 'xxs';
const isLargeViewport = breakpoint === 's';
const i18n = useInternalI18n('top-navigation');

// ButtonDropdown supports checkbox items but we don't support these in TopNavigation. Shown an error in development mode
Expand Down Expand Up @@ -98,23 +97,14 @@ export default function InternalTopNavigation({
className={clsx(styles['top-navigation'], {
[styles.virtual]: isVirtual,
[styles.hidden]: isVirtual,
[styles.narrow]: isNarrowViewport,
[styles.medium]: isMediumViewport,
})}
>
<div className={styles['padding-box']}>
{showIdentity && (
<div className={clsx(styles.identity, !identity.logo && styles['no-logo'])}>
<a className={styles['identity-link']} href={identity.href} onClick={onIdentityClick}>
{identity.logo && (
<img
role="img"
src={identity.logo?.src}
alt={identity.logo?.alt}
className={clsx(styles.logo, {
[styles.narrow]: isNarrowViewport,
})}
/>
<img role="img" src={identity.logo?.src} alt={identity.logo?.alt} className={styles.logo} />
)}
{showTitle && <span className={styles.title}>{identity.title}</span>}
</a>
Expand All @@ -135,11 +125,7 @@ export default function InternalTopNavigation({
className={clsx(
styles['utility-wrapper'],
styles['utility-type-button'],
styles['utility-type-button-link'],
{
[styles.narrow]: isNarrowViewport,
[styles.medium]: isMediumViewport,
}
styles['utility-type-button-link']
)}
data-utility-special="search"
>
Expand All @@ -163,69 +149,45 @@ export default function InternalTopNavigation({
(_utility, i) =>
isVirtual || !responsiveState.hideUtilities || responsiveState.hideUtilities.indexOf(i) === -1
)
.map((utility, i) => {
const hideText = !!responsiveState.hideUtilityText;
const isLast = (isVirtual || !showMenuTrigger) && i === utilities.length - 1;
const offsetRight = isLast && isLargeViewport ? 'xxl' : isLast ? 'l' : undefined;

return (
<div
key={i}
className={clsx(
styles['utility-wrapper'],
styles[`utility-type-${utility.type}`],
utility.type === 'button' && styles[`utility-type-button-${utility.variant ?? 'link'}`],
{
[styles.narrow]: isNarrowViewport,
[styles.medium]: isMediumViewport,
}
)}
data-utility-index={i}
data-utility-hide={`${hideText}`}
>
<Utility hideText={hideText} definition={utility} offsetRight={offsetRight} />
</div>
);
})}

{isVirtual &&
utilities.map((utility, i) => {
const hideText = !responsiveState.hideUtilityText;
const isLast = !showMenuTrigger && i === utilities.length - 1;
const offsetRight = isLast && isLargeViewport ? 'xxl' : isLast ? 'l' : undefined;

return (
.map((utility, i) => (
<div
key={i}
className={clsx(
styles['utility-wrapper'],
styles[`utility-type-${utility.type}`],
utility.type === 'button' && styles[`utility-type-button-${utility.variant ?? 'link'}`],
{
[styles.narrow]: isNarrowViewport,
[styles.medium]: isMediumViewport,
}
utility.type === 'button' && styles[`utility-type-button-${utility.variant ?? 'link'}`]
)}
data-utility-index={i}
data-utility-hide={`${hideText}`}
data-utility-hide={`${!!responsiveState.hideUtilityText}`}
>
<Utility hideText={hideText} definition={utility} offsetRight={offsetRight} />
<Utility hideText={!!responsiveState.hideUtilityText} definition={utility} />
</div>
);
})}
))}

{isVirtual &&
utilities.map((utility, i) => (
<div
key={i}
className={clsx(
styles['utility-wrapper'],
styles[`utility-type-${utility.type}`],
utility.type === 'button' && styles[`utility-type-button-${utility.variant ?? 'link'}`]
)}
data-utility-index={i}
data-utility-hide={`${!responsiveState.hideUtilityText}`}
>
<Utility hideText={!responsiveState.hideUtilityText} definition={utility} />
</div>
))}

{showMenuTrigger && (
<div
className={clsx(styles['utility-wrapper'], styles['utility-type-menu-dropdown'], {
[styles.narrow]: isNarrowViewport,
[styles.medium]: isMediumViewport,
})}
className={clsx(styles['utility-wrapper'], styles['utility-type-menu-dropdown'])}
data-utility-special="menu-trigger"
>
<ButtonTrigger
expanded={overflowMenuOpen}
onClick={toggleOverflowMenu}
offsetRight="l"
ref={!isVirtual ? overflowMenuTriggerRef : undefined}
>
{i18n('i18nStrings.overflowMenuTriggerText', i18nStrings?.overflowMenuTriggerText)}
Expand Down
9 changes: 5 additions & 4 deletions src/top-navigation/parts/utility.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React from 'react';
import clsx from 'clsx';

import { InternalButton } from '../../button/internal';
import { isLinkItem } from '../../button-dropdown/utils/utils';
Expand Down Expand Up @@ -33,7 +32,7 @@ export default function Utility({ hideText, definition, offsetRight }: UtilityPr
checkSafeUrl('TopNavigation', definition.href);
if (definition.variant === 'primary-button') {
return (
<span className={styles[`offset-right-${offsetRight}`]}>
<span>
<InternalButton
variant="primary"
href={definition.href}
Expand All @@ -53,7 +52,7 @@ export default function Utility({ hideText, definition, offsetRight }: UtilityPr
<>
{' '}
<span
className={clsx(styles['utility-button-external-icon'], styles[`offset-right-${offsetRight}`])}
className={styles['utility-button-external-icon']}
aria-label={definition.externalIconAriaLabel}
role={definition.externalIconAriaLabel ? 'img' : undefined}
>
Expand All @@ -69,7 +68,7 @@ export default function Utility({ hideText, definition, offsetRight }: UtilityPr
} else {
// Link
return (
<span className={styles[`offset-right-${offsetRight}`]}>
<span>
<InternalLink
variant="top-navigation"
href={definition.href}
Expand Down Expand Up @@ -120,7 +119,9 @@ export default function Utility({ hideText, definition, offsetRight }: UtilityPr
items={items}
title={shouldShowTitle ? title : ''}
ariaLabel={ariaLabel}
expandToViewport={true}
offsetRight={offsetRight}
className={styles['utility-menu-dropdown-button']}
>
{!shouldHideText && definition.text}
</MenuDropdown>
Expand Down
Loading
Loading