feat: Update sidebar, fix mobile responsiveness #5209

mikemurray · 2019-06-12T05:51:55Z

Impact: minor
Type: feature

Issue

Sidebar design is out-of-date with current designs
Sidebar isn't closable on mobile

Solution

Update sidebar to current design spec
Fix mobile responsiveness
Make it easier to create toolbars without having to re-implement the same logic over and over

Breaking changes

uiState.isLeftSidebarOpen renamed to uiState.isPrimarySidebarOpen.

Testing

Run the app, and login into the admin
See the new sidebar
Click around
Resize to mobile
Open / close sidebar
With the sidebar open, click on a menu item
The sidebar should close and navigate you to that page
Resize back to desktop
Continue to use the sidebar

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

…unctions Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

mikemurray · 2019-06-12T05:53:19Z

Not quite done, but close.

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

…AppBars Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

mikemurray · 2019-06-13T22:40:19Z

@rymorgan @cassytaylor Sort order for items:

The top-level items are sorted
Shop, Payment, Taxes, Shipping are manually sorted
Other items will in whatever order under the sorted ones

kieckhafer · 2019-06-13T22:51:02Z

imports/client/ui/components/PrimaryAppBar/PrimaryAppBar.js

+import Typography from "@material-ui/core/Typography";
+import withStyles from "@material-ui/core/styles/withStyles";
+import MenuIcon from "mdi-material-ui/Menu";
+import { UIContext } from "../../context/UIContext";


It seems like this is something that will be used all over the app, should we do some sort of alias for UIContext?

Maybe, but all of these dashboard components needs to be pulled out of this app a component library. Also, this context is not to be used as a MobX store to store random data. It's only meant for basic UI state to ensure the components for the operator-ui function where ever they may live.

For now, my logic is, only use this context for components of the operator experience (sidebar, main AppBars, core layouts, etc). Don't make use of this context in your components in other packages. Make a component in the in /imports/client/components that makes use of the context. At least until this is in a better home.

kieckhafer · 2019-06-13T22:52:18Z

imports/client/ui/components/PrimaryAppBar/PrimaryAppBar.js

+      {({ isMobile, isPrimarySidebarOpen, onTogglePrimarySidebar }) => {
+        const toolbarClassName = classNames({
+          // Add padding to the right when the primary sidebar is open,
+          // only if we're on desktop. On Mobile the sidebar floats over


lowercase mobile

kieckhafer · 2019-06-13T22:57:58Z

imports/client/ui/layouts/Dashboard.js

+  "container": {
+    display: "flex"
+  },
+  "leftSidebarOpen": {
    paddingLeft: 280 + (theme.spacing.unit * 2)


Same as other comment, we should make 280 a variable

kieckhafer · 2019-06-13T23:02:31Z

imports/plugins/core/catalog/client/components/publishControls.js

 import Typography from "@material-ui/core/Typography";
 import withStyles from "@material-ui/core/styles/withStyles";
 import { i18next } from "/client/api";
+import PrimaryAppBar from "../../../../../client/ui/components/PrimaryAppBar/PrimaryAppBar";


Can we make this "/imports/client/ui/components/PrimaryAppBar instead of all the ..'s?

kieckhafer

@reactioncommerce/design @mikemurray Should there be some sort of down arrow here? It took me a few seconds to realize this was a drop down menu, I initially thought that the other settings hadn't loaded correctly.

This could be because I'm so used to the old version though.

kieckhafer · 2019-06-13T23:17:55Z

This isn't an issue with this PR, but I want to mention it here for further thought / discussion.

I feel the multiple save buttons are confusing, it's hard to know which button / buttons need to be clicked in order to save my changes.

kieckhafer

A few very minor comments, overall this looks / works great!

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

mikemurray · 2019-06-14T18:02:57Z

@kieckhafer I don't think it needs it. There are only a few links on the top level, and you'll know when it's open once you see all of the nested items. Also, this is an evolution of the menu, but the revolution may end up moving those options to their own menu altogether.

I defer to what @reactioncommerce/design ultimately wants though.

mikemurray · 2019-06-14T18:10:51Z

@kieckhafer ready for a re-review

rymorgan · 2019-06-14T21:56:53Z

I don't think the settings expandable navigation needs the arrow.

This isn't an issue with this PR, but I want to mention it here for further thought / discussion.

I feel the multiple save buttons are confusing, it's hard to know which button / buttons need to be clicked in order to save my changes.

I agree with the above but think we need to think about this as the system evolves. Maybe whenever it's in the card it needs to say 'update'. Also in general in the design system, the convention should buttons whose actions pertain to the overall view should be on the page or in the top bar. Buttons that pertain to only the info in the card should be in the card.

mikemurray added 15 commits June 11, 2019 22:18

feat: show shop name with logo

fa7b3bd

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: add ui context provider

4d42431

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: add toolbar component for main content areas

516722e

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: provide ui context, and handle responsive layout the with MUI f…

b0ef7ad

…unctions Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

fix: update sidebar layout to match current design direction

8529fa9

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: update product admin route with material icon and priority

38966f9

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: use PrimaryAppBar component

7c30265

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: update route config

ed97b42

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: update mui theme

58aae53

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: update route config

54268d1

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: use PrimaryAppBar component

41e9e52

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: update route config

d0555b0

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: use PrimaryAppBar component

edd6c83

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: update route config

3249019

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

fix: sidebar open state when adjusting from desktop -> mobile -> desktop

b2945d3

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

mikemurray added 14 commits June 12, 2019 19:35

refactor: use FAB button

7e8c8f2

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

refactor: default settings to closed

cd55cf3

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

refactor: use const

6c956e7

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

refactor: move title display to the toolbar

fc6f0c9

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

fix: adjust responsive behavior

870d4a2

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

refactor: remove is mobile as it was unnecessary

dc2d285

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

refactor: remove is mobile as it was unnecessary

5506cb4

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

refactor: remove is mobile as it was unnecessary

e61a6e6

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: add isMobile to UIContext

a64fde0

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

fix: responsiveness with sidebar open states

70b8b94

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

fix: use correct prop type

7ffcb75

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

fix: sticky scroll header

f8e1ea1

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: improve UX with active states, and closing on link click

1e1a690

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

feat: update various theme values for a better mobile experience for …

bc7dcf0

…AppBars Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

mikemurray added 4 commits June 13, 2019 14:51

feat: sort routes by priority

edc1905

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

refactor: update route priorities

a8f6b5e

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

fix: open first settings route on expand only on desktop

cab8039

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

fix: highlight only if settings isn't open

0dac2ff

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

mikemurray requested review from kieckhafer, rymorgan and cassytaylor June 13, 2019 22:36

mikemurray marked this pull request as ready for review June 13, 2019 22:40

mikemurray changed the title ~~[WIP] feat: Update sidebar, fix mobile responsiveness~~ feat: Update sidebar, fix mobile responsiveness Jun 13, 2019

kieckhafer reviewed Jun 13, 2019

View reviewed changes

kieckhafer requested changes Jun 13, 2019

View reviewed changes

mikemurray added 3 commits June 14, 2019 10:24

fix: typo in comment

af3daaa

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

refactor: use variables for drawer width padding property

8fab577

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

refactor: use absolute path for import

49193ed

Signed-off-by: Mike Murray <hellomikemurray@gmail.com>

rymorgan approved these changes Jun 14, 2019

View reviewed changes

kieckhafer approved these changes Jun 17, 2019

View reviewed changes

kieckhafer merged commit 7157471 into develop Jun 17, 2019

kieckhafer deleted the feat-mikemurray-update-sidebar-ui branch June 17, 2019 20:02

jeffcorpuz mentioned this pull request Jul 2, 2019

Release v2.0.0 rc.12 #5259

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Update sidebar, fix mobile responsiveness #5209

feat: Update sidebar, fix mobile responsiveness #5209

mikemurray commented Jun 12, 2019

mikemurray commented Jun 12, 2019

mikemurray commented Jun 13, 2019

kieckhafer Jun 13, 2019

mikemurray Jun 14, 2019

kieckhafer Jun 13, 2019

kieckhafer Jun 13, 2019

kieckhafer Jun 13, 2019

kieckhafer left a comment

kieckhafer commented Jun 13, 2019

kieckhafer left a comment

mikemurray commented Jun 14, 2019

mikemurray commented Jun 14, 2019

rymorgan commented Jun 14, 2019 •

edited

Loading

feat: Update sidebar, fix mobile responsiveness #5209

feat: Update sidebar, fix mobile responsiveness #5209

Conversation

mikemurray commented Jun 12, 2019

Issue

Solution

Breaking changes

Testing

mikemurray commented Jun 12, 2019

mikemurray commented Jun 13, 2019

kieckhafer Jun 13, 2019

Choose a reason for hiding this comment

mikemurray Jun 14, 2019

Choose a reason for hiding this comment

kieckhafer Jun 13, 2019

Choose a reason for hiding this comment

kieckhafer Jun 13, 2019

Choose a reason for hiding this comment

kieckhafer Jun 13, 2019

Choose a reason for hiding this comment

kieckhafer left a comment

Choose a reason for hiding this comment

kieckhafer commented Jun 13, 2019

kieckhafer left a comment

Choose a reason for hiding this comment

mikemurray commented Jun 14, 2019

mikemurray commented Jun 14, 2019

rymorgan commented Jun 14, 2019 • edited Loading

rymorgan commented Jun 14, 2019 •

edited

Loading