Skip to content
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

mobile global nav #447

Merged
merged 3 commits into from
Feb 29, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions app/assets/icons/carrot - large.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 7 additions & 0 deletions app/assets/icons/list - large.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 5 additions & 0 deletions app/assets/icons/x - large.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions app/components/Button/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const Button = ({children, action, tooltip, icon, className}: ButtonProps) => {
return (
<button className={classes} onClick={action} data-tooltip={tooltip}>
{children}
{icon && icon}
Copy link
Collaborator

Choose a reason for hiding this comment

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

just add it to the children, rather than adding an extra property just for this. Also, {icon && icon} is exactly the same as {icon} (I'm actually supprised that the linter didn't complain)

</button>
)
}
Expand Down
118 changes: 118 additions & 0 deletions app/components/Nav/Mobile/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import React from 'react'
import {Link} from '@remix-run/react'
import AISafetyIcon from '~/components/icons-generated/Aisafety'
import {ListLarge} from '~/components/icons-generated'
import {XLarge} from '~/components/icons-generated'
import {CarrotLarge} from '~/components/icons-generated'
import OpenBookIcon from '~/components/icons-generated/OpenBook'
import BotIcon from '~/components/icons-generated/Bot'
import {NavProps} from '~/components/Nav'
import Button from '~/components/Button'
import '../nav.css'
import './navMobile.css'
import {sortFuncs} from '~/routes/tags.$'
import {questionUrl, tagsUrl, tagUrl} from '~/routesMapper'
import {TOCItem} from '~/routes/questions.toc'
export const MobileNav = ({toc, categories}: NavProps) => {
const [showMenu, setShowMenu] = React.useState(false)
const [showArticles, setShowArticles] = React.useState(false)
const toggleMenu = () => {
setShowMenu(!showMenu)
setShowArticles(false)
}
console.log('toc', toc)
return (
<header className="top-header">
{!showArticles && (
<>
<nav className="top-nav">
Copy link
Collaborator

Choose a reason for hiding this comment

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

how much of this could be handled with css in the normal Nav component?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure I follow, can you please explain what you mean?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is basically the same as the current Nav, with a bit of things moved around. So you could have a mobile-only class that you add to the show menu button and a desktop-only one on the searchbar, which (with a couple of extra css settings) will allow you to have all of these in the same component, rather than duplicating them. The menu in theory could also be done with css, but that might be tricky. Still - there would then only be a few new items, which will make things simpler

Copy link
Collaborator

Choose a reason for hiding this comment

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

I only just noticed that I didn't send this comment...

Copy link
Member Author

Choose a reason for hiding this comment

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

do you still want this done...?

Copy link
Collaborator

Choose a reason for hiding this comment

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

preferably. But this certainly still needs to be connected to the actual page

<Link to="/" className="top-logo">
<AISafetyIcon />
</Link>
{!showMenu ? (
<ListLarge className={'menu-button'} onClick={toggleMenu} />
) : (
<XLarge className={'menu-button'} onClick={toggleMenu} />
)}
</nav>
{showMenu && (
<div className="mobile-menu">
<Button
action={() => setShowArticles(!showArticles)}
className="secondary full-width space-between"
icon={<CarrotLarge />}
>
<p className={'composite-elements'}>
<OpenBookIcon className={'icon-margin'} />
Articles
</p>
</Button>
<Button action="https://xkcd.com/285/" className="secondary full-width flex-start">
<p className={'composite-elements'}>
<BotIcon className={'icon-margin'} />
Stampy chatbot
</p>
</Button>
</div>
)}
</>
)}
{showArticles && (
<>
<nav className="articles-header">
<CarrotLarge className={'back-icon'} onClick={() => setShowArticles(false)} />
<p className={'composite-elements'}>
<OpenBookIcon className={'icon-margin'} />
Articles
</p>
<XLarge className={'menu-button'} onClick={toggleMenu} />
</nav>

<div className={'articles-mobile'}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should reuse the ArticlesDropdown component. A bit of extra css there should handle transforming it from a grid into a list

<div className="default-bold margin-bottom">Introductory</div>
{toc
.filter((item) => item.category === 'Introductory')
.map((item: TOCItem) => (
<Link
key={`${item.pageid}-${item.title}`}
className="articles-entry"
to={questionUrl(item)}
>
{item.title}
</Link>
))}
<div className="default-bold margin-bottom top-margin">Advanced</div>
{toc
.filter((item) => item.category === 'Advanced')
.map((item: TOCItem) => (
<Link
key={`${item.pageid}-${item.title}`}
to={questionUrl(item)}
className="articles-entry"
>
{item.title}
</Link>
))}
<div className="default-bold margin-bottom top-margin">Browse by category</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

what are the classes margin-bottom and top-margin? For bottom padding we want to use the class "padding-bottom-[number]". Maybe I will create padding-top classes and even margin classes too...

{categories
?.sort(sortFuncs['by number of questions'])
.slice(0, 12)
.map((tag) => (
<Link
key={tag.rowId}
className="articles-dropdown-teal-entry articles-entry"
to={tagUrl(tag)}
>
{tag.name}
</Link>
))}
<Button action={tagsUrl()} className="secondary">
<span> Browse all categories</span>
</Button>
</div>
</>
)}
</header>
)
}
export default MobileNav
59 changes: 59 additions & 0 deletions app/components/Nav/Mobile/navMobile.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
@media only screen and (max-width: 640px) {
.top-header {
padding: 0;
}
.top-nav {
justify-content: space-between;
padding: var(--spacing-20) var(--spacing-24);
Copy link
Contributor

Choose a reason for hiding this comment

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

The nav was supposed to be based on height (using 80px) rather than padding. If you want to use padding can you just round to the nearest one, so either 16px or 24px? And remove the --spacing-20 var

}
.top-logo {
padding: 0;
}
.menu-button {
cursor: pointer;
}
.composite-elements {
display: flex;
align-items: center;
}
.space-between {
justify-content: space-between;
}
.flex-start {
justify-content: flex-start;
}
.mobile-menu > .button {
border-radius: 0;
font-size: 16px;
Copy link
Contributor

Choose a reason for hiding this comment

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

never write any CSS for fonts! Assign whichever is the right class based on the Figma, or just h1 or h2 tag.

}
.mobile-menu > .button:first-child {
border-bottom: 0;
}
.icon-margin {
margin-right: var(--spacing-8);
}
.articles-header {
display: flex;
justify-content: space-between;
align-items: center;
padding: var(--spacing-24);
border-bottom: 1px solid var(--colors-cool-grey-200);
}
.back-icon {
cursor: pointer;
transform: rotate(180deg);
}
.articles-entry {
display: block;
margin-bottom: var(--spacing-16);
}
.articles-mobile {
padding: var(--spacing-24);
}
.margin-bottom {
margin-bottom: var(--spacing-24);
}
.top-margin {
margin-top: var(--spacing-40);
}
}
7 changes: 7 additions & 0 deletions app/components/icons-generated/CarrotLarge.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import type {SVGProps} from 'react'
const SvgCarrotLarge = (props: SVGProps<SVGSVGElement>) => (
<svg xmlns="http://www.w3.org/2000/svg" width={24} height={24} fill="none" {...props}>
<path stroke="#AFB7C2" d="m8 3 9 9-9 9" />
</svg>
)
export default SvgCarrotLarge
9 changes: 9 additions & 0 deletions app/components/icons-generated/ListLarge.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type {SVGProps} from 'react'
const SvgListLarge = (props: SVGProps<SVGSVGElement>) => (
<svg xmlns="http://www.w3.org/2000/svg" width={24} height={24} fill="none" {...props}>
<g stroke="#788492" strokeLinecap="round">
<path d="M22 12H2M22 7H2M22 17H2" />
</g>
</svg>
)
export default SvgListLarge
10 changes: 10 additions & 0 deletions app/components/icons-generated/XLarge.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import type {SVGProps} from 'react'
const SvgXLarge = (props: SVGProps<SVGSVGElement>) => (
<svg xmlns="http://www.w3.org/2000/svg" width={24} height={24} fill="none" {...props}>
<path
fill="#AFB7C2"
d="M4.646 18.646a.5.5 0 0 0 .708.708L12 12.707l6.646 6.647a.5.5 0 0 0 .708-.708L12.707 12l6.647-6.646a.5.5 0 0 0-.708-.708L12 11.293 5.354 4.646a.5.5 0 1 0-.708.708L11.293 12z"
/>
</svg>
)
export default SvgXLarge
3 changes: 3 additions & 0 deletions app/components/icons-generated/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export {default as Arrow} from './Arrow'
export {default as Bot} from './Bot'
export {default as BottomEclipse} from './BottomEclipse'
export {default as BoxArrow} from './BoxArrow'
export {default as CarrotLarge} from './CarrotLarge'
export {default as Chatbot} from './Chatbot'
export {default as Clipboard} from './Clipboard'
export {default as Code} from './Code'
Expand All @@ -24,6 +25,7 @@ export {default as GroupTopEcplise} from './GroupTopEcplise'
export {default as Hide} from './Hide'
export {default as Like} from './Like'
export {default as Link} from './Link'
export {default as ListLarge} from './ListLarge'
export {default as MagnifyingGlass} from './MagnifyingGlass'
export {default as Magnifying} from './Magnifying'
export {default as OpenBook} from './OpenBook'
Expand All @@ -45,4 +47,5 @@ export {default as ThumbUpLarge} from './ThumbUpLarge'
export {default as ThumbUp} from './ThumbUp'
export {default as Undo} from './Undo'
export {default as Users} from './Users'
export {default as XLarge} from './XLarge'
export {default as X} from './X'
1 change: 1 addition & 0 deletions app/newRoot.css
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
--spacing-8: 8px;
--spacing-12: 12px;
--spacing-16: 16px;
--spacing-20: 20px;
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this, sorry!

--spacing-24: 24px;
--spacing-32: 32px;
--spacing-40: 40px;
Expand Down
Loading
Loading