-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
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
[SpeedDial] Rework part of the logic #17301
Merged
oliviertassinari
merged 6 commits into
mui:master
from
hashwin:hashwin/persistent_tooltip
Sep 15, 2019
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
77 changes: 77 additions & 0 deletions
77
docs/src/pages/components/speed-dial/OpenIconSpeedDial.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| import React from 'react'; | ||
| import { makeStyles, createStyles, Theme } from '@material-ui/core/styles'; | ||
| import Button from '@material-ui/core/Button'; | ||
| import SpeedDial from '@material-ui/lab/SpeedDial'; | ||
| import SpeedDialIcon from '@material-ui/lab/SpeedDialIcon'; | ||
| import SpeedDialAction from '@material-ui/lab/SpeedDialAction'; | ||
| import FileCopyIcon from '@material-ui/icons/FileCopyOutlined'; | ||
| import SaveIcon from '@material-ui/icons/Save'; | ||
| import PrintIcon from '@material-ui/icons/Print'; | ||
| import ShareIcon from '@material-ui/icons/Share'; | ||
| import DeleteIcon from '@material-ui/icons/Delete'; | ||
| import EditIcon from '@material-ui/icons/Edit'; | ||
|
|
||
| const useStyles = makeStyles((theme: Theme) => | ||
| createStyles({ | ||
| root: { | ||
| height: 380, | ||
| transform: 'translateZ(0px)', | ||
| flexGrow: 1, | ||
| }, | ||
| speedDial: { | ||
| position: 'absolute', | ||
| bottom: theme.spacing(2), | ||
| right: theme.spacing(2), | ||
| }, | ||
| }), | ||
| ); | ||
|
|
||
| const actions = [ | ||
| { icon: <FileCopyIcon />, name: 'Copy' }, | ||
| { icon: <SaveIcon />, name: 'Save' }, | ||
| { icon: <PrintIcon />, name: 'Print' }, | ||
| { icon: <ShareIcon />, name: 'Share' }, | ||
| { icon: <DeleteIcon />, name: 'Delete' }, | ||
| ]; | ||
|
|
||
| export default function OpenIconSpeedDial() { | ||
| const classes = useStyles(); | ||
| const [open, setOpen] = React.useState(false); | ||
| const [hidden, setHidden] = React.useState(false); | ||
|
|
||
| const handleVisibility = () => { | ||
| setHidden(prevHidden => !prevHidden); | ||
| }; | ||
|
|
||
| const handleOpen = () => { | ||
| setOpen(true); | ||
| }; | ||
|
|
||
| const handleClose = () => { | ||
| setOpen(false); | ||
| }; | ||
|
|
||
| return ( | ||
| <div className={classes.root}> | ||
| <Button onClick={handleVisibility}>Toggle Speed Dial</Button> | ||
| <SpeedDial | ||
| ariaLabel="SpeedDial openIcon example" | ||
| className={classes.speedDial} | ||
| hidden={hidden} | ||
| icon={<SpeedDialIcon openIcon={<EditIcon />} />} | ||
| onClose={handleClose} | ||
| onOpen={handleOpen} | ||
| open={open} | ||
| > | ||
| {actions.map(action => ( | ||
| <SpeedDialAction | ||
| key={action.name} | ||
| icon={action.icon} | ||
| tooltipTitle={action.name} | ||
| onClick={handleClose} | ||
| /> | ||
| ))} | ||
| </SpeedDial> | ||
| </div> | ||
| ); | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might it be more helpful to have a click handler that shows how to identify which action has been clicked, before it calls onClose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could help, it would make the demo a bit more verbose, but could teach people a valuable pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From (past) personal experience, it's the kind of thing React beginners struggle with. I think it might be worth a little verbosity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think that people can leverage index based handler when displaying data from the server, for instance, in the case of inbox (R.I.P.), the most frequent contacts. Or in the case of Google Calendar, I would expect each action to have a named custom handler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how to action this comment. I would expect most people to have a fixed set of options (not using an array). However, our demo is using an array for the sake of minimizing the demo length. I believe these two objectives contradict themselves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the point is that the click handler does not handle each action differently which should be the most common use case. Otherwise why have a speed dial if every action does the same (which only closes at the moment). I would expect that you want to dispatch some action. It would be nice if this demo would handle that case.
At least this is how I understood @mbrookes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok thanks, this sounds good 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eps1lon Yeah, that's the crux of it. I took it from @oliviertassinari's comment that in real usage you wouldn't use a mapped array, but would compose each action component. If each then calls a different click handler, the issue goes away.
I had pictured a common handler that dispatches a different event according to the calling action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just use an inline handler that captures the action. A mapped array is the first iteration you would do. The rest is premature optimization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to merge, I think that we can apply the handler changes in another pull request :).