Skip to content

add padding bottom to avoid overlap with banners #2454

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

Closed
wants to merge 14 commits into from
Closed

add padding bottom to avoid overlap with banners #2454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
9320f43
add padding bottom to avoid overlap with banners
jomcarvajal Mar 31, 2025
f496a13
Merge branch 'main' into CORE-802-content-overlaps
jomcarvajal Mar 31, 2025
3748c3e
run linter
jomcarvajal Mar 31, 2025
a871808
Merge branch 'CORE-802-content-overlaps' of github.com:openstax/rex-w…
jomcarvajal Mar 31, 2025
4c2e7b1
update snapshots
jomcarvajal Mar 31, 2025
cea1295
Merge branch 'main' into CORE-802-content-overlaps
staxly[bot] Apr 3, 2025
1e7fe1c
Merge branch 'main' into CORE-802-content-overlaps
staxly[bot] Apr 4, 2025
519fed5
Merge branch 'main' into CORE-802-content-overlaps
staxly[bot] Apr 8, 2025
94df4f5
Merge branch 'main' into CORE-802-content-overlaps
staxly[bot] Apr 15, 2025
e35386f
Merge branch 'main' into CORE-802-content-overlaps
staxly[bot] Apr 15, 2025
97732a4
Merge branch 'main' into CORE-802-content-overlaps
staxly[bot] Apr 21, 2025
5e50d41
Merge branch 'main' into CORE-802-content-overlaps
staxly[bot] Apr 21, 2025
c43cf9b
Merge branch 'main' into CORE-802-content-overlaps
staxly[bot] Apr 28, 2025
5655f13
Merge branch 'main' into CORE-802-content-overlaps
staxly[bot] Apr 29, 2025
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: 8 additions & 0 deletions src/app/content/__snapshots__/routes.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ Array [
overflow: visible;
z-index: 19;
top: 12rem;
padding-bottom: 0;
}

.c3:focus-within {
Expand Down Expand Up @@ -723,6 +724,13 @@ Array [
left: 0;
z-index: 21;
top: 11.3rem;
padding-bottom: 0;
}
}

@media screen and (max-width:50em) {
.c1 {
padding-bottom: 0;
}
}

Expand Down
24 changes: 22 additions & 2 deletions src/app/content/components/Page/PageToasts.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import {
bookBannerDesktopMiniHeight,
bookBannerMobileMiniHeight,
contentWrapperMaxWidth,
toastHeightMediumRes,
toastHeightRes,
toastHeightSmallRes,
toolbarMobileSearchWrapperHeight,
topbarDesktopHeight,
topbarMobileHeight,
Expand All @@ -18,7 +21,7 @@ import { contentWrapperAndNavWidthBreakpoint, contentWrapperWidthBreakpoint } fr
import { ToastProps } from '../../../notifications/components/ToastNotifications/Toast';

export const desktopSearchFailureTop = bookBannerDesktopMiniHeight + topbarDesktopHeight;
export const getMobileSearchFailureTop = ({mobileToolbarOpen}: {mobileToolbarOpen: boolean}) => mobileToolbarOpen
export const getMobileSearchFailureTop = ({ mobileToolbarOpen }: { mobileToolbarOpen: boolean }) => mobileToolbarOpen
? bookBannerMobileMiniHeight + topbarMobileHeight + toolbarMobileSearchWrapperHeight
: bookBannerMobileMiniHeight + topbarMobileHeight;

Expand All @@ -28,6 +31,7 @@ export const ToastContainerWrapper = styled.div`
overflow: visible;
z-index: ${theme.zIndex.contentNotifications - 1};
top: ${desktopSearchFailureTop}rem;
padding-bottom: ${props => props.visibleToasts ? `${toastHeightRes * props.visibleToasts}rem` : '0'};

@media screen and ${contentWrapperAndNavWidthBreakpoint} {
max-width: calc(100vw - ((100vw - ${contentWrapperMaxWidth}rem) / 2) - ${verticalNavbarMaxWidth}rem);
Expand All @@ -44,6 +48,17 @@ export const ToastContainerWrapper = styled.div`
left: 0;
z-index: ${theme.zIndex.contentNotifications + 1};
top: ${getMobileSearchFailureTop}rem;
padding-bottom: ${(props: { visibleToasts: number; }) => props.visibleToasts
? `${toastHeightMediumRes * props.visibleToasts}rem`
: '0'
};
`)}

${theme.breakpoints.mobileMedium(css`
padding-bottom: ${(props: { visibleToasts: number; }) => props.visibleToasts
? `${toastHeightSmallRes * props.visibleToasts}rem`
: '0'
};
`)}
`;

Expand All @@ -63,7 +78,12 @@ const PageToasts = (props: ToastProps | {}) => {
const mobileToolbarOpen = useSelector(mobileToolbarOpenSelector);

return (
<ToastContainerWrapper role='alertdialog' {...props} mobileToolbarOpen={mobileToolbarOpen}>
<ToastContainerWrapper
role='alertdialog'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that with the role "alertdialog" it does not matter that the content overlaps.
It was previously "alert". The difference is that "alertdialog" is something that is supposed to be dismissed -- like these. That role change was a change I made recently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should cancel the card CORE-802 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be reviewed along with CORE-804 (the card my PR was attached to). I think they're effectively the same.
I don't think it's wrong to make room for the boxes, but I think if we want to do that, we might just want to have them appear in a reserved space rather than absolute-positioned.

{...props}
mobileToolbarOpen={mobileToolbarOpen}
visibleToasts={toasts?.length}
>
{toasts ? <ToastNotifications toasts={toasts} /> : null}
</ToastContainerWrapper>
);
Expand Down
24 changes: 24 additions & 0 deletions src/app/content/components/Page/__snapshots__/PageToasts.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ exports[`PageToasts matches snapshot 1`] = `
overflow: visible;
z-index: 19;
top: 12rem;
padding-bottom: 0;
}

@media screen and (max-width:90em) {
Expand All @@ -29,6 +30,13 @@ exports[`PageToasts matches snapshot 1`] = `
left: 0;
z-index: 21;
top: 11.3rem;
padding-bottom: 0;
}
}

@media screen and (max-width:50em) {
.c0 {
padding-bottom: 0;
}
}

Expand Down Expand Up @@ -131,6 +139,7 @@ exports[`PageToasts matches snapshot with toasts when mobile toolbar is open 1`]
overflow: visible;
z-index: 19;
top: 12rem;
padding-bottom: 4rem;
}

@media (max-width:75em) {
Expand Down Expand Up @@ -177,6 +186,13 @@ exports[`PageToasts matches snapshot with toasts when mobile toolbar is open 1`]
left: 0;
z-index: 21;
top: 16.6rem;
padding-bottom: 6rem;
}
}

@media screen and (max-width:50em) {
.c0 {
padding-bottom: 8rem;
}
}

Expand Down Expand Up @@ -335,6 +351,7 @@ exports[`PageToasts matches snapshots with toasts 1`] = `
overflow: visible;
z-index: 19;
top: 12rem;
padding-bottom: 4rem;
}

@media (max-width:75em) {
Expand Down Expand Up @@ -381,6 +398,13 @@ exports[`PageToasts matches snapshots with toasts 1`] = `
left: 0;
z-index: 21;
top: 11.3rem;
padding-bottom: 6rem;
}
}

@media screen and (max-width:50em) {
.c0 {
padding-bottom: 8rem;
}
}

Expand Down
16 changes: 16 additions & 0 deletions src/app/content/components/__snapshots__/Content.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1254,6 +1254,7 @@ Array [
overflow: visible;
z-index: 19;
top: 12rem;
padding-bottom: 0;
}

.c42 {
Expand Down Expand Up @@ -2628,6 +2629,13 @@ li[aria-label="Current Page"] .c61 {
left: 0;
z-index: 21;
top: 11.3rem;
padding-bottom: 0;
}
}

@media screen and (max-width:50em) {
.c78 {
padding-bottom: 0;
}
}

Expand Down Expand Up @@ -6001,6 +6009,7 @@ Array [
overflow: visible;
z-index: 19;
top: 12rem;
padding-bottom: 0;
}

.c42 {
Expand Down Expand Up @@ -7272,6 +7281,13 @@ li[aria-label="Current Page"] .c61 {
left: 0;
z-index: 21;
top: 11.3rem;
padding-bottom: 0;
}
}

@media screen and (max-width:50em) {
.c78 {
padding-bottom: 0;
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/app/content/components/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,8 @@ export const mainContentBackground = '#fff';
export const maxContentGutter = 6;
export const contentWrapperMaxWidth = contentTextWidth + sidebarDesktopWidth + verticalNavbarMaxWidth;

export const toastHeightRes = 4;
export const toastHeightMediumRes = 6;
export const toastHeightSmallRes = 8;

export const defaultTheme = 'blue' as BookWithOSWebData['theme'];
Loading