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.

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

Refactored Header/MobileNav.jsx with all Todos #3312

Closed
wants to merge 1 commit into from
Closed

Refactored Header/MobileNav.jsx with all Todos #3312

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
73 changes: 43 additions & 30 deletions client/modules/IDE/components/Header/MobileNav.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import React, { useContext, useMemo, useState } from 'react';
import React, { useContext, useState } from 'react';
import styled from 'styled-components';
import { useDispatch, useSelector } from 'react-redux';
import { useTranslation } from 'react-i18next';
import { Link } from 'react-router-dom';
import { sortBy } from 'lodash';
import classNames from 'classnames';
import PropTypes from 'prop-types';

import { ParentMenuContext } from '../../../../components/Nav/contexts';
import NavBar from '../../../../components/Nav/NavBar';
import { useMenuProps } from '../../../../components/Nav/NavDropdownMenu';
Expand Down Expand Up @@ -35,6 +37,7 @@ import { setLanguage } from '../../actions/preferences';
import Overlay from '../../../App/components/Overlay';
import ProjectName from './ProjectName';
import CollectionCreate from '../../../User/components/CollectionCreate';
import { selectRootFile } from '../../selectors/files';

const Nav = styled(NavBar)`
background: ${prop('MobilePanel.default.background')};
Expand Down Expand Up @@ -200,34 +203,36 @@ const LanguageSelect = styled.div`
}
`;

const MobileNav = () => {
const MobileNav = ({ title }) => {
const project = useSelector((state) => state.project);
const user = useSelector((state) => state.user);

const { t } = useTranslation();
// console.log('The title: ', title);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally, we want to isolate changes in each pull request to tie as much to the original issue as possible. In most scenarios, we recommend not having pull requests build on top of another and create dependencies on each other.

Copy link
Author

Choose a reason for hiding this comment

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

By update i meant i started from scratch but worked on the same problem, just with a different idea in mind.
i didn't used any code from previous pull request.


// const { t } = useTranslation();

const editorLink = useSelector(selectSketchPath);
const pageName = useWhatPage();

// TODO: remove the switch and use a props like mobileTitle <Nav layout=“dashboard” mobileTitle={t(‘Login’)} />
function resolveTitle() {
switch (pageName) {
case 'login':
return t('LoginView.Login');
case 'signup':
return t('LoginView.SignUp');
case 'account':
return t('AccountView.Settings');
case 'examples':
return t('Nav.File.Examples');
case 'myStuff':
return 'My Stuff';
default:
return project.name;
}
}

const title = useMemo(resolveTitle, [pageName, project.name]);
// function resolveTitle() {
// switch (pageName) {
// case 'login':
// return t('LoginView.Login');
// case 'signup':
// return t('LoginView.SignUp');
// case 'account':
// return t('AccountView.Settings');
// case 'examples':
// return t('Nav.File.Examples');
// case 'myStuff':
// return 'My Stuff';
// default:
// return project.name;
// }
// }

// const title = useMemo(resolveTitle, [pageName, project.name]);

const Logo = AsteriskIcon;
return (
Expand All @@ -244,7 +249,7 @@ const MobileNav = () => {
)}
</Title>
{/* check if the user is in login page */}
{pageName === 'login' || pageName === 'signup' ? (
{pageName === 'LoginView.Login' || pageName === 'LoginView.SignUp' ? (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Replacing hardcoded strings is a great move here!

// showing the CrossIcon
<Options>
<div>
Expand All @@ -256,7 +261,7 @@ const MobileNav = () => {
) : (
// Menus for other pages
<Options>
{pageName === 'myStuff' && <StuffMenu />}
{pageName === 'My Stuff' && <StuffMenu />}
{user.authenticated ? (
<AccountMenu />
) : (
Expand All @@ -266,7 +271,7 @@ const MobileNav = () => {
</Link>
</div>
)}
{pageName === 'home' ? (
{pageName === 'home' || pageName === project.name ? (
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could probably look more lik {pageName === project.name ? ( to remove the hardcoded string!

<MoreMenu />
) : (
<div>
Expand All @@ -281,6 +286,14 @@ const MobileNav = () => {
);
};

MobileNav.propTypes = {
title: PropTypes.string
};

MobileNav.defaultProps = {
title: 'p5.js Web Editor'
};

const StuffMenu = () => {
const { isOpen, handlers } = useMenuProps('stuff');
const { newSketch } = useSketchActions();
Expand Down Expand Up @@ -341,9 +354,8 @@ const AccountMenu = () => {

const MoreMenu = () => {
// TODO: use selectRootFile selector
const rootFile = useSelector(
(state) => state.files.filter((file) => file.name === 'root')[0]
);
// DONE
Copy link
Collaborator

Choose a reason for hiding this comment

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

If a comment is addressed, the original one can simply be deleted!

const rootFile = useSelector(selectRootFile);
const language = useSelector((state) => state.preferences.language);

const dispatch = useDispatch();
Expand All @@ -367,8 +379,8 @@ const MoreMenu = () => {
{isLanguageModalVisible && (
<Overlay
// TODO: add translations
title="Select Language"
ariaLabel="Select Language"
title={t('selectLanguage', { defaultValue: 'Select Language' })}
ariaLabel={t('selectLanguage', { defaultValue: 'Select Language' })}
closeOverlay={() => setIsLanguageModalVisible(false)}
>
<LanguageSelect>
Expand Down Expand Up @@ -420,7 +432,8 @@ const MoreMenu = () => {
{t('Nav.Sketch.AddFolder')}
</NavMenuItem>
{/* TODO: Add Translations */}
<b>Settings</b>
{/* DONE */}
<b>{t('settings', { defaultValue: 'Settings' })}</b>
<NavMenuItem
onClick={() => {
dispatch(openPreferences());
Expand Down
12 changes: 8 additions & 4 deletions client/modules/IDE/components/Header/Nav.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@ import { CmControllerContext } from '../../pages/IDEView';
import MobileNav from './MobileNav';
import useIsMobile from '../../hooks/useIsMobile';

const Nav = ({ layout }) => {
const Nav = ({ layout, mobileTitle }) => {
const isMobile = useIsMobile();

// console.log('The updated mobile title is: ', mobileTitle);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We would ideally want to remove commented out logs!


return isMobile ? (
<MobileNav />
<MobileNav title={mobileTitle} />
) : (
<NavBar>
<LeftLayout layout={layout} />
Expand All @@ -45,11 +47,13 @@ const Nav = ({ layout }) => {
};

Nav.propTypes = {
layout: PropTypes.oneOf(['dashboard', 'project'])
layout: PropTypes.oneOf(['dashboard', 'project']),
mobileTitle: PropTypes.string
};

Nav.defaultProps = {
layout: 'project'
layout: 'project',
mobileTitle: 'p5.js Web Editor'
};

const LeftLayout = (props) => {
Expand Down
13 changes: 7 additions & 6 deletions client/modules/IDE/hooks/useWhatPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,21 @@ import { useLocation } from 'react-router-dom';
*/
const useWhatPage = () => {
const username = useSelector((state) => state.user.username);
const project = useSelector((state) => state.project);
const { pathname } = useLocation();

const pageName = useMemo(() => {
const myStuffPattern = new RegExp(
`(/${username}/(sketches/?$|collections|assets)/?)`
);

if (myStuffPattern.test(pathname)) return 'myStuff';
else if (pathname === '/login') return 'login';
else if (pathname === '/signup') return 'signup';
else if (pathname === '/account') return 'account';
if (myStuffPattern.test(pathname)) return 'My Stuff';
else if (pathname === '/login') return 'LoginView.Login';
else if (pathname === '/signup') return 'LoginView.SignUp';
else if (pathname === '/account') return 'AccountView.Settings';
else if (pathname === '/p5/collections' || pathname === '/p5/sketches')
return 'examples';
return 'home';
return 'Nav.File.Examples';
return project.name || 'home';
}, [pathname, username]);

return pageName;
Expand Down
5 changes: 4 additions & 1 deletion client/modules/Legal/pages/Legal.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { remSize } from '../../../theme';
import Loader from '../../App/components/loader';
import Nav from '../../IDE/components/Header/Nav';
import PolicyContainer from '../components/PolicyContainer';
import { useWhatPage } from '../../IDE/hooks';

const StyledTabList = styled.nav`
display: flex;
Expand All @@ -24,6 +25,8 @@ const StyledTabList = styled.nav`

function Legal({ policyFile, title }) {
const { t } = useTranslation();
const pageName = useWhatPage();
const pagetitle = String(t(pageName));
const [isLoading, setIsLoading] = useState(true);
const [policy, setPolicy] = useState('');

Expand All @@ -40,7 +43,7 @@ function Legal({ policyFile, title }) {
<Helmet>
<title>p5.js Web Editor | {title}</title>
</Helmet>
<Nav layout="dashboard" />
<Nav layout="dashboard" mobileTitle={pagetitle} />
<StyledTabList className="dashboard-header__switcher">
<ul className="dashboard-header__tabs">
<RouterTab to="/privacy-policy">{t('Legal.PrivacyPolicy')}</RouterTab>
Expand Down
5 changes: 4 additions & 1 deletion client/modules/User/pages/AccountView.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import Nav from '../../IDE/components/Header/Nav';
import ErrorModal from '../../IDE/components/ErrorModal';
import Overlay from '../../App/components/Overlay';
import Toast from '../../IDE/components/Toast';
import { useWhatPage } from '../../IDE/hooks';

function SocialLoginPanel() {
const { t } = useTranslation();
Expand Down Expand Up @@ -44,6 +45,8 @@ function SocialLoginPanel() {

function AccountView() {
const { t } = useTranslation();
const pageName = useWhatPage();
const pagetitle = String(t(pageName));

const location = useLocation();
const queryParams = parse(location.search);
Expand All @@ -59,7 +62,7 @@ function AccountView() {
</Helmet>
<Toast />

<Nav layout="dashboard" />
<Nav layout="dashboard" mobileTitle={pagetitle} />

{showError && (
<Overlay
Expand Down
10 changes: 9 additions & 1 deletion client/modules/User/pages/CollectionView.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
import React from 'react';
import { useTranslation } from 'react-i18next';
import { useParams } from 'react-router-dom';
import Nav from '../../IDE/components/Header/Nav';
import RootPage from '../../../components/RootPage';
import Collection from '../components/Collection';
import { useWhatPage } from '../../IDE/hooks';

const CollectionView = () => {
const params = useParams();
const { t } = useTranslation();
const pageName = useWhatPage();
const pagetitle = String(t(pageName));

console.log('the pagename: ', pageName);
console.log('the pagename: ', t(pageName));

return (
<RootPage>
<Nav layout="dashboard" />
<Nav layout="dashboard" mobileTitle={pagetitle} />
<Collection
collectionId={params.collection_id}
username={params.username}
Expand Down
5 changes: 4 additions & 1 deletion client/modules/User/pages/DashboardView.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,13 @@ import DashboardTabSwitcherPublic, {
TabKey
} from '../components/DashboardTabSwitcher';
import useIsMobile from '../../IDE/hooks/useIsMobile';
import { useWhatPage } from '../../IDE/hooks';

const DashboardView = () => {
const isMobile = useIsMobile();
const { t } = useTranslation();
const pageName = useWhatPage();
const pagetitle = String(t(pageName));

const params = useParams();
const location = useLocation();
Expand Down Expand Up @@ -117,7 +120,7 @@ const DashboardView = () => {

return (
<RootPage fixedHeight="100%">
<Nav layout="dashboard" />
<Nav layout="dashboard" mobileTitle={pagetitle} />

<main className="dashboard-header">
<div className="dashboard-header__header">
Expand Down
6 changes: 5 additions & 1 deletion client/modules/User/pages/EmailVerificationView.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,13 @@ import { useTranslation } from 'react-i18next';
import { verifyEmailConfirmation } from '../actions';
import RootPage from '../../../components/RootPage';
import Nav from '../../IDE/components/Header/Nav';
import { useWhatPage } from '../../IDE/hooks';

const EmailVerificationView = () => {
const { t } = useTranslation();
const pageName = useWhatPage();
const pagetitle = String(t(pageName));

const location = useLocation();
const dispatch = useDispatch();
const browserHistory = useHistory();
Expand Down Expand Up @@ -38,7 +42,7 @@ const EmailVerificationView = () => {

return (
<RootPage>
<Nav layout="dashboard" />
<Nav layout="dashboard" mobileTitle={pagetitle} />
<div className="form-container">
<Helmet>
<title>{t('EmailVerificationView.Title')}</title>
Expand Down
6 changes: 5 additions & 1 deletion client/modules/User/pages/LoginView.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@ import LoginForm from '../components/LoginForm';
import SocialAuthButton from '../components/SocialAuthButton';
import Nav from '../../IDE/components/Header/Nav';
import RootPage from '../../../components/RootPage';
import { useWhatPage } from '../../IDE/hooks';

function LoginView() {
const { t } = useTranslation();
const pageName = useWhatPage();
const pagetitle = String(t(pageName));

return (
<RootPage>
<Nav layout="dashboard" />
<Nav layout="dashboard" mobileTitle={pagetitle} />
<main className="form-container">
<Helmet>
<title>{t('LoginView.Title')}</title>
Expand Down
6 changes: 5 additions & 1 deletion client/modules/User/pages/NewPasswordView.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,13 @@ import NewPasswordForm from '../components/NewPasswordForm';
import { validateResetPasswordToken } from '../actions';
import Nav from '../../IDE/components/Header/Nav';
import RootPage from '../../../components/RootPage';
import { useWhatPage } from '../../IDE/hooks';

function NewPasswordView() {
const { t } = useTranslation();
const pageName = useWhatPage();
const pagetitle = String(t(pageName));

const params = useParams();
const resetPasswordToken = params.reset_password_token;
const resetPasswordInvalid = useSelector(
Expand All @@ -30,7 +34,7 @@ function NewPasswordView() {
});
return (
<RootPage>
<Nav layout="dashboard" />
<Nav layout="dashboard" mobileTitle={pagetitle} />
<div className={newPasswordClass}>
<Helmet>
<title>{t('NewPasswordView.Title')}</title>
Expand Down
Loading