-
Notifications
You must be signed in to change notification settings - Fork 200
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
[IRN-5018][BpkNavigationBar][BpkBottomSheet] Add handling for long title text #3393
Changes from all commits
31ddb69
dd898db
428256c
17b72f6
bd2ebf1
b289a27
600844f
23fb2b9
945a874
7945dd8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,29 @@ const DefaultExample = () => ( | |
</div> | ||
); | ||
|
||
const LongTitleTextExample = () => ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Demo of misbehaving header at Nav Bar level - now fixed and inherited by Bottom Sheet |
||
<div className={getClassNames('bpk-navigation-bar-story')}> | ||
<BpkNavigationBar | ||
id="test" | ||
title="Backpack navigation bar long title example" | ||
leadingButton={ | ||
<BpkNavigationBarIconButton | ||
onClick={action('back clicked')} | ||
icon={ArrowIconWithRtl} | ||
label="back" | ||
/> | ||
} | ||
trailingButton={ | ||
<BpkNavigationBarIconButton | ||
onClick={action('close clicked')} | ||
icon={CloseIcon} | ||
label="close" | ||
/> | ||
} | ||
/> | ||
</div> | ||
); | ||
|
||
const OnDarkExample = () => ( | ||
<div className={getClassNames('bpk-navigation-bar-story')}> | ||
<BpkNavigationBar | ||
|
@@ -241,16 +264,17 @@ const VisualTestExample = () => ( | |
<OnDarkExample /> | ||
<WithLinksOnDarkExample /> | ||
</div> | ||
) | ||
); | ||
|
||
export { | ||
DefaultExample, | ||
LongTitleTextExample, | ||
OnDarkExample, | ||
LeadingIconOnlyExample, | ||
TrailingIconOnlyExample, | ||
WithLinksExample, | ||
WithLogoExample, | ||
WithLinksOnDarkExample, | ||
StickyExample, | ||
VisualTestExample | ||
VisualTestExample, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,8 +30,6 @@ | |
animation-timing-function: ease-in-out; | ||
} | ||
|
||
$close-button-width: tokens.bpk-spacing-lg() * 2; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was unused so removed |
||
|
||
.bpk-bottom-sheet { | ||
z-index: tokens.$bpk-zindex-modal; | ||
width: 100%; | ||
|
@@ -114,8 +112,13 @@ $close-button-width: tokens.bpk-spacing-lg() * 2; | |
position: sticky; | ||
top: 0; | ||
z-index: tokens.$bpk-zindex-tooltip - 1; | ||
padding: tokens.bpk-spacing-sm() 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding additional padding to the heading based on BottomSheet design (avoiding adding to Nav Bar directly to avoid all other consumers gaining height) |
||
|
||
@include borders.bpk-border-bottom-sm(tokens.$bpk-line-day); | ||
@include utils.bpk-themeable-property( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
background-color, | ||
--bpk-navigation-bar-background-color, | ||
tokens.$bpk-surface-default-day | ||
); | ||
} | ||
|
||
@keyframes slide-up { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,7 +24,7 @@ import BpkCloseButton from '../../bpk-component-close-button'; | |
// @ts-expect-error Untyped import. See `decisions/imports-ts-suppressions.md`. | ||
import { BpkButtonLink } from '../../bpk-component-link'; | ||
import BpkNavigationBar from "../../bpk-component-navigation-bar"; | ||
import BpkText, { TEXT_STYLES } from "../../bpk-component-text/src/BpkText"; | ||
import { TEXT_STYLES } from "../../bpk-component-text/src/BpkText"; | ||
import { BpkDialogWrapper, cssModules } from "../../bpk-react-utils"; | ||
|
||
import STYLES from './BpkBottomSheet.module.scss'; | ||
|
@@ -87,63 +87,63 @@ const BpkBottomSheet = ({ | |
const dialogClassName = getClassName( | ||
'bpk-bottom-sheet', | ||
wide && 'bpk-bottom-sheet--wide' | ||
); | ||
); | ||
|
||
return <BpkBreakpoint query={BREAKPOINTS.ABOVE_MOBILE}> | ||
{(isAboveMobile: boolean) => | ||
return <BpkBreakpoint query={BREAKPOINTS.ABOVE_MOBILE}> | ||
{(isAboveMobile: boolean) => | ||
<BpkDialogWrapper | ||
ariaLabelledby={ariaLabelledby} | ||
dialogClassName={dialogClassName} | ||
id={id} | ||
isOpen={isOpen} | ||
onClose={( | ||
arg0?: TouchEvent | MouseEvent | KeyboardEvent | SyntheticEvent<HTMLDialogElement, Event>, | ||
arg1?: { | ||
source: 'ESCAPE' | 'DOCUMENT_CLICK'; | ||
}) => handleClose( isAboveMobile ? 0 : animationTimeout, arg0, arg1)} | ||
exiting={exiting} | ||
transitionClassNames={{ | ||
appear: getClassName('bpk-bottom-sheet--appear'), | ||
appearActive: getClassName('bpk-bottom-sheet--appear-active'), | ||
exit: getClassName('bpk-bottom-sheet--exit') | ||
}} | ||
closeOnEscPressed={closeOnEscPressed} | ||
closeOnScrimClick={closeOnScrimClick} | ||
timeout={{appear: animationTimeout, exit: isAboveMobile ? 0 : animationTimeout}} | ||
ariaLabelledby={ariaLabelledby} | ||
dialogClassName={dialogClassName} | ||
id={id} | ||
isOpen={isOpen} | ||
onClose={( | ||
arg0?: TouchEvent | MouseEvent | KeyboardEvent | SyntheticEvent<HTMLDialogElement, Event>, | ||
arg1?: { | ||
source: 'ESCAPE' | 'DOCUMENT_CLICK'; | ||
}) => handleClose(isAboveMobile ? 0 : animationTimeout, arg0, arg1)} | ||
exiting={exiting} | ||
transitionClassNames={{ | ||
appear: getClassName('bpk-bottom-sheet--appear'), | ||
appearActive: getClassName('bpk-bottom-sheet--appear-active'), | ||
exit: getClassName('bpk-bottom-sheet--exit') | ||
}} | ||
closeOnEscPressed={closeOnEscPressed} | ||
closeOnScrimClick={closeOnScrimClick} | ||
timeout={{ appear: animationTimeout, exit: isAboveMobile ? 0 : animationTimeout }} | ||
> | ||
<> | ||
<header className={getClassName('bpk-bottom-sheet--header')}> | ||
<BpkNavigationBar | ||
id={headingId} | ||
title={title && | ||
<BpkText id={headingId} textStyle={TEXT_STYLES.label1} tagName="h2">{title}</BpkText> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can now just pass the text and options for which element and style we want (which lets the component perform the text overflow for us) |
||
} | ||
leadingButton={ | ||
<BpkCloseButton | ||
label={closeLabel} | ||
onClick={( | ||
arg0?: TouchEvent | MouseEvent | KeyboardEvent | SyntheticEvent<HTMLDialogElement, Event>, | ||
arg1?: { | ||
source: 'ESCAPE' | 'DOCUMENT_CLICK'; | ||
}) => handleClose( isAboveMobile ? 0 : animationTimeout, arg0, arg1)} | ||
/> | ||
} | ||
trailingButton={ | ||
actionText && onAction ? ( | ||
<BpkButtonLink | ||
onClick={onAction} | ||
> | ||
{actionText} | ||
</BpkButtonLink> | ||
) : | ||
null | ||
} | ||
/> | ||
</header> | ||
<div className={getClassName('bpk-bottom-sheet--content')}>{children}</div> | ||
</> | ||
</BpkDialogWrapper> | ||
} | ||
<> | ||
<header className={getClassName('bpk-bottom-sheet--header')}> | ||
<BpkNavigationBar | ||
id={headingId} | ||
title={title} | ||
titleTextStyle={TEXT_STYLES.label1} | ||
titleTagName={title ? "h2" : "span"} | ||
leadingButton={ | ||
<BpkCloseButton | ||
label={closeLabel} | ||
onClick={( | ||
arg0?: TouchEvent | MouseEvent | KeyboardEvent | SyntheticEvent<HTMLDialogElement, Event>, | ||
arg1?: { | ||
source: 'ESCAPE' | 'DOCUMENT_CLICK'; | ||
}) => handleClose(isAboveMobile ? 0 : animationTimeout, arg0, arg1)} | ||
/> | ||
} | ||
trailingButton={ | ||
actionText && onAction ? ( | ||
<BpkButtonLink | ||
onClick={onAction} | ||
> | ||
{actionText} | ||
</BpkButtonLink> | ||
) : | ||
null | ||
} | ||
/> | ||
</header> | ||
<div className={getClassName('bpk-bottom-sheet--content')}>{children}</div> | ||
</> | ||
</BpkDialogWrapper> | ||
} | ||
</BpkBreakpoint> | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,7 +52,7 @@ exports[`BpkBottomSheet renders correctly with action props 1`] = ` | |
class="bpk-navigation-bar__title bpk-navigation-bar__title--default" | ||
> | ||
<span | ||
class="bpk-text bpk-text--heading-5" | ||
class="bpk-text bpk-text--label-1" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting these have now changed as this could lead me to believe these used to be wrong! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Although since the span was empty it wouldn't have been a noticeable difference either way. |
||
id="bpk-bottom-sheet-heading-my-bottom-sheet-bpk-navigation-bar-title" | ||
/> | ||
</span> | ||
|
@@ -131,7 +131,7 @@ exports[`BpkBottomSheet renders correctly with minimum prop 1`] = ` | |
class="bpk-navigation-bar__title bpk-navigation-bar__title--default" | ||
> | ||
<span | ||
class="bpk-text bpk-text--heading-5" | ||
class="bpk-text bpk-text--label-1" | ||
id="bpk-bottom-sheet-heading-my-bottom-sheet-bpk-navigation-bar-title" | ||
/> | ||
</span> | ||
|
@@ -200,7 +200,7 @@ exports[`BpkBottomSheet renders correctly with wide prop 1`] = ` | |
class="bpk-navigation-bar__title bpk-navigation-bar__title--default" | ||
> | ||
<span | ||
class="bpk-text bpk-text--heading-5" | ||
class="bpk-text bpk-text--label-1" | ||
id="bpk-bottom-sheet-heading-my-bottom-sheet-bpk-navigation-bar-title" | ||
/> | ||
</span> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Demo of misbehaving header at Bottom Sheet level - now fixed