Skip to content

Banner overflow #49

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

Merged
merged 5 commits into from
Jun 24, 2025
Merged

Banner overflow #49

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1540eeb
fix: add z-index to demo notification for improved visibility
debs-obrien Jun 24, 2025
d652294
feat: add DemoBanner component for demo notifications
debs-obrien Jun 24, 2025
411c2db
feat: enhance layout and styling across components for improved UI co…
debs-obrien Jun 24, 2025
57b6b2a
fix: adjust dimensions and padding for improved layout consistency ac…
debs-obrien Jun 24, 2025
1284d8f
Update movies-app/components/DemoBanner/index.js
debs-obrien Jun 24, 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
27 changes: 27 additions & 0 deletions movies-app/components/DemoBanner/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import React from 'react';

const DemoBanner = () => (
<>
<div className="demo-banner">
This is a demo intended for testing. Data and images are provided by{' '}
<a href="https://www.themoviedb.org">TMDB</a>.
</div>
<style jsx>{`
.demo-banner {
position: fixed;
top: 0;
left: 0;
width: 100%;
height: 30px;
background-color: #ffee5b;
color: #444;
padding: 0.75rem 1rem;
text-align: center;
border-bottom: 1px solid #cc0;
z-index: 1000;
}
`}</style>
</>
);

export default DemoBanner;
17 changes: 11 additions & 6 deletions movies-app/components/Menu/SectionHeading/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,17 @@ const SectionHeading = ({
{children}
</h2>
<style jsx>{`
font-weight: ${theme.typography.fontWeightBold};
font-size: 1.25rem;
color: var(--palette-text-primary);
text-transform: uppercase;
letter-spacing: -0.5px;
margin: 0 0 1rem 1rem;
h2 {
font-weight: ${theme.typography.fontWeightBold};
font-size: 1.25rem;
color: var(--palette-text-primary);
text-transform: uppercase;
letter-spacing: -0.5px;
margin: 0 0 1rem 1rem;
padding-top: 1rem;
position: relative;
}

h2:not(:first-child) {
margin-top: 4rem;
}
Expand Down
3 changes: 0 additions & 3 deletions movies-app/components/Menu/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@

import { useSelector } from 'react-redux';

import Logo from 'components/Logo';
import SectionHeading from './SectionHeading';
import MenuItem from './MenuItem';
import MenuItemLink from './MenuItemLink';
Expand Down Expand Up @@ -51,7 +50,6 @@ const renderGenres = (genres, selectedMenuItemName, closeMenu = null) => {
};

const Menu = ({
isMobile,
closeMenu,
...rest
}) => {
Expand All @@ -62,7 +60,6 @@ const Menu = ({
return (
<>
<nav {...rest}>
{!isMobile && <Logo />}
<SectionHeading>Discover</SectionHeading>
{renderStaticCategories(staticCategories, selectedMenuItemName, closeMenu)}
<SectionHeading>Genres</SectionHeading>
Expand Down
10 changes: 6 additions & 4 deletions movies-app/components/UI/AppBar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const AppBar = ({
.app-bar {
position: fixed;
top: 30px;
left: auto;
left: 0;
right: 0;
width: 100%;
z-index: ${theme.zIndex.appBar};
Expand All @@ -30,7 +30,7 @@ const AppBar = ({
}

.toolbar {
min-height: 64px;
min-height: 46px;
padding: 0 24px;
display: flex;
align-items: center;
Expand All @@ -39,14 +39,16 @@ const AppBar = ({

@media ${theme.mediaQueries.small} {
.toolbar {
min-height: 56px;
min-height: 42px;
padding: 0 16px;
}
}

@media ${theme.mediaQueries.smaller} {
.app-bar {
background-color: transparent;
background-color: var(--palette-background-paper);
width: 100%;
left: 0;
}
}
`}</style>
Expand Down
31 changes: 30 additions & 1 deletion movies-app/containers/AppHeader/BurgerHeader/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,22 @@ import HamburgerButton from 'components/UI/HamburgerButton';
import SearchBar from 'containers/SearchBar';
import DarkModeToggle from 'containers/DarkModeToggle';
import TheUser from 'containers/TheUser';
import { LOGO_IMAGE_PATH } from 'utils/constants/image-paths';

const BurgerHeader = ({ openMenu }) => (
<>
<AppBar>
<HamburgerButton onClick={openMenu} />
<div className='left-section'>
<HamburgerButton onClick={openMenu} />
<div className='logo-container'>
Copy link
Preview

Copilot AI Jun 24, 2025

Choose a reason for hiding this comment

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

[nitpick] Same logo-container/logo-img duplication here. Extract into a reusable component or shared styles to keep the code DRY.

Copilot uses AI. Check for mistakes.

<img
className='logo-img'
width='56'
height='56'
src={LOGO_IMAGE_PATH}
alt='movie ticket' />
</div>
</div>
<div className='sticky-bar-widgets-container'>
<SearchBar id='mobile' />
<DarkModeToggle
Expand All @@ -19,6 +30,24 @@ const BurgerHeader = ({ openMenu }) => (
</div>
</AppBar>
<style jsx>{`
.left-section {
display: flex;
align-items: center;
}

.logo-container {
margin-left: 25px;
display: flex;
align-items: center;
}

.logo-img {
max-height: 56px;
width: auto;
margin-top: -10px;
margin-bottom: -10px;
}

.sticky-bar-widgets-container {
display: flex;
align-items: center;
Expand Down
6 changes: 3 additions & 3 deletions movies-app/containers/SearchBar/Form/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@ const Form = React.forwardRef(({
border: 1px solid var(--palette-secondary-main);
width: ${opened ? '30rem' : '2rem'};
cursor: ${opened ? 'auto' : 'pointer'};
padding: 2rem;
height: 2rem;
padding: 1.6rem;
height: 1.7rem;
outline: none;
border-radius: 100px;
transition: width ${theme.transitions.duration.standard}ms ${theme.transitions.easing.easeInOut};
}

@media ${theme.mediaQueries.large} {
.form {
padding: 1.5rem;
padding: 1.3rem;
border: 1px solid transparent;
background-color: var(--palette-secondary-main);
}
Expand Down
2 changes: 1 addition & 1 deletion movies-app/containers/SearchBar/MagnifierButton/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const MagnifierButton = ({
{/* MEMO: inspired by https://web.dev/prefers-color-scheme/#use-currentcolor-for-inline-svgs */}
<SearchIcon
fill='currentColor'
width='1.125em' />
width='1em' />
</button>
<style jsx>{`
.magnifier-button {
Expand Down
2 changes: 1 addition & 1 deletion movies-app/containers/Sidebar/SidebarInnerWrapper/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const SidebarInnerWrapper = ({ children }) => (
flex-direction: column;
width: 25rem;
padding: 2rem;
margin-top: 4rem;
padding-top: 8rem;
border-right: 1px solid var(--palette-divider);
}
`}</style>
Expand Down
24 changes: 18 additions & 6 deletions movies-app/containers/Sidebar/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,24 @@ import LazyMenu from 'parts/LazyMenu';
import StickyBox from 'utils/hocs/StickyBox';

const Sidebar = ({ className }) => (
<StickyBox className={className}>
<SidebarInnerWrapper>
{/* TODO: introduce a layout shifting */}
<LazyMenu />
</SidebarInnerWrapper>
</StickyBox>
<div className="sidebar-container">
<StickyBox className={className}>
<SidebarInnerWrapper>
<div className="menu-container">
<LazyMenu />
</div>
</SidebarInnerWrapper>
</StickyBox>
<style jsx>{`
.sidebar-container {
position: relative;
z-index: 0;
}
.menu-container {
padding-top: 2rem;
}
`}</style>
</div>
);

export default Sidebar;
3 changes: 0 additions & 3 deletions movies-app/pages/_document.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@ class MyDocument extends Document {
{/* <script async src="https://www.google-analytics.com/analytics.js"></script> */}
</Head>
<body className={CLASS_NAMES.LIGHT}>
<div style={{ position: 'fixed', backgroundColor: "#ffee5b", color: "#444", padding: "0.75rem 1rem", textAlign: "center", borderBottom: "1px solid #cc0", width: "100%", height: '30px' }}>This is a demo intended for testing. Data and images are provided by <a href="https://www.themoviedb.org">TMDB</a>.</div>
<div style={{ height: "50px"}} />

{/* MEMO: inspired by https://github.com/donavon/use-dark-mode#that-flash */}
<Script>
{() => {
Expand Down
4 changes: 2 additions & 2 deletions movies-app/parts/Layout/ContentWrapper/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ const ContentWrapper = ({

@media ${theme.mediaQueries.larger} {
.content-wrapper {
padding-top: 72px;
padding-top: 100px;
padding-bottom: 6rem;
}
}

@media ${theme.mediaQueries.large} {
.content-wrapper {
padding-top: 72px;
padding-top: 100px;
Comment on lines +25 to +32
Copy link
Preview

Copilot AI Jun 24, 2025

Choose a reason for hiding this comment

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

[nitpick] This padding-top value is duplicated in two media queries. Extract the value into a constant or shared style to reduce duplication and ease future adjustments.

Copilot uses AI. Check for mistakes.

padding-bottom: 4rem;
}
}
Expand Down
58 changes: 47 additions & 11 deletions movies-app/parts/Layout/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

import { useEffect } from 'react';
import { useDispatch } from 'react-redux';

Expand All @@ -8,6 +7,8 @@ import MyHead from 'components/MyHead';
import SearchBar from 'containers/SearchBar';
import DarkModeToggle from 'containers/DarkModeToggle';
import TheUser from 'containers/TheUser';
import DemoBanner from 'components/DemoBanner';
import { LOGO_IMAGE_PATH } from 'utils/constants/image-paths';
import MainWrapper from './MainWrapper';
import ContentWrapper from './ContentWrapper';
import init from 'actions/init';
Expand All @@ -30,6 +31,7 @@ const Layout = ({
return (
<>
<MyHead />
<DemoBanner />
{/**
* TODO: it could be more efficient in using markups.
* children is duplicated -> looks like it affects the performance (a little).
Expand All @@ -48,19 +50,52 @@ const Layout = ({
<Media greaterThan='sm'>
<MainWrapper theme={theme}>
<Sidebar />
<div className='desktop-widgets-container'>
<SearchBar id='desktop' />
<DarkModeToggle
id='desktop'
className='left-margin' />
<TheUser />
<div className='desktop-header-container'>
<div className='logo-container'>
<img
className='logo-img'
width='56'
height='56'
src={LOGO_IMAGE_PATH}
alt='movie ticket' />
</div>
Comment on lines +54 to +61
Copy link
Preview

Copilot AI Jun 24, 2025

Choose a reason for hiding this comment

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

[nitpick] The logo-container and logo-img styles are duplicated in both Layout and BurgerHeader. Consider abstracting this logo block into a shared component or CSS module to avoid repetition.

Suggested change
<div className='logo-container'>
<img
className='logo-img'
width='56'
height='56'
src={LOGO_IMAGE_PATH}
alt='movie ticket' />
</div>
<LogoBlock />

Copilot uses AI. Check for mistakes.

<div className='desktop-widgets-container'>
<SearchBar id='desktop' />
<DarkModeToggle
id='desktop'
className='left-margin' />
<TheUser />
</div>
</div>
<style jsx>{`
.desktop-widgets-container {
position: absolute;
top: 0;
.desktop-header-container {
position: fixed;
top: 30px;
left: 0;
right: 0;
padding: 2rem;
width: 100%;
display: flex;
justify-content: space-between;
align-items: center;
padding: 0.5rem 2rem;
z-index: ${theme.zIndex.appBar + 10};
background-color: var(--palette-background-paper);
}

.logo-container {
display: flex;
align-items: center;
margin-left: 15px;
}

.logo-img {
max-height: 56px;
width: auto;
margin-top: -10px;
margin-bottom: -10px;
}

.desktop-widgets-container {
display: flex;
align-items: center;
}
Expand All @@ -75,6 +110,7 @@ const Layout = ({
</MainWrapper>
</Media>
</MediaContextProvider>
<DemoBanner />
</>
);
};
Expand Down
3 changes: 2 additions & 1 deletion movies-app/parts/SummarySectionHeading/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ const SummarySectionHeading = ({
<h3 className={clsx('summary-section-heading', className)}>{children}</h3>
<style jsx>{`
.summary-section-heading {
margin-bottom: 1rem;
margin-top: 1rem;
margin-bottom: 1.5rem;
color: var(--palette-text-primary);
font-size: 1.5rem;
font-weight: ${theme.typography.fontWeightBold};
Expand Down