[docs] Switch order of snackbar buttons in demos #37389
Conversation
Switch left and right, so left comes first and right comes later when reading left to right Signed-off-by: Jannis Hell <[email protected]>
Netlify deploy previewhttps://deploy-preview-37389--material-ui.netlify.app/ Bundle size report |
ZeeshanTamboli
added
docs
|
The buttons to spawn the Snackbar on the left of the screen are positioned on the right. The buttons to show it on the right, are on the left. It feels more natural to put the button also on the side where the snackbar will appear. Same goes for the example further down: Right is on the left side, left is further right... A more positive example is the positioned tooltips:
Left is left, up is up, right is right and down is down - that's what feels logical and natural to me. |
|
Maybe it makes more sense to align it in a plus shape like we have at positioned tooltips? I'm happy to come up with a proposed change if you think that would clear things up - also for consistency it might be nice to align with the way tooltips shows it. |
|
I'll leave the decision to @danilo-leal, @zanivan. |
|
The current flow is meant to be clockwise, starting from top-center—and it makes the right buttons stay on the left side 😅 As I see, the best way to fix this is to make it like the tooltips, as @Primajin said. This way, users can navigate clockwise, and also see the left buttons on the left and vice versa—and keep it consistent with we already have on the tooltip page. |
|
Alright cool, then I'll look into that and ping you once the change is made 👍🏻 |
Switch left and right, so left comes first and right comes later when reading left to right Signed-off-by: Jannis Hell <[email protected]>
|
How about this:
|
|
Can you help me understand what the failing test is trying to tell me? When I run it locally I get: > yarn docs:typescript:formatted
yarn run v1.22.19
$ cross-env BABEL_ENV=development babel-node --extensions ".tsx,.ts,.js" ./docs/scripts/formattedTSDemos
C:\[...]\material-ui\packages\typescript-to-proptypes\src\index.ts:1
import * as ts from 'typescript';
^^^^^^
SyntaxError: Cannot use import statement outside a module
at internalCompileFunction (node:internal/vm:73:18)
at wrapSafe (node:internal/modules/cjs/loader:1175:20)
at Module._compile (node:internal/modules/cjs/loader:1219:27)
at Module._compile (C:\[...]\material-ui\node_modules\pirates\lib\index.js:136:24)
at Module._extensions..js (node:internal/modules/cjs/loader:1309:10)
at Object.newLoader [as .ts] (C:\[...]\material-ui\node_modules\pirates\lib\index.js:141:7)
at Module.load (node:internal/modules/cjs/loader:1113:32)
at Module._load (node:internal/modules/cjs/loader:960:12)
at Module.require (node:internal/modules/cjs/loader:1137:19)
at require (node:internal/modules/helpers:121:18)
Node.js v20.2.0
error Command failed with exit code 1. |
|
@Primajin That's strange. I tried locally using the same version of Node.js @zanivan @danilo-leal Can you review? |
|
It's looking good! Just tweaked the demo a bit, to be more similar to the one we have for tooltips. |
|
@zanivan Feel free to approve and merge the PR if it looks good. |
There was a problem hiding this comment.
Looks good! I fixed some code in 89356cc to use textAlign instead of flex column positioning.
ZeeshanTamboli
changed the title
ZeeshanTamboli
changed the title
|
Thanks folks! 🙌🏻 |
|
I have done a follow-up in #37657 this PR broke the layout in mobile: 
|
|
Ahh thanks, I thought I had tested everything on mobile but maybe some of the later changes that came in broke it 😱 |
|
I forgot to test for mobile screens. Thanks for the follow up! |
Before
https://deploy-preview-37387--material-ui.netlify.app/material-ui/react-snackbar/#positioned-snackbars
After
https://deploy-preview-37389--material-ui.netlify.app/material-ui/react-snackbar/#positioned-snackbars
explanation
I find it more logical when we go from left to right, that also left comes first and then right comes next.