[SpeedDial] Allow a tooltip placement prop by seanchambo · Pull Request #12244 · mui/material-ui

seanchambo · 2018-07-23T02:22:55Z

Allows adding a placement prop for the tooltip of the speed dial action.
I'm making my speed dial go horizontal which makes the tooltip positioning wrong

Closes #10895

mbrookes · 2018-07-23T09:42:28Z

@seanchambo What technique are you using for your horizontal speed-dial? There's an open issue for it (#10895)...

seanchambo · 2018-07-24T03:54:04Z

I am just applying a class to the root and actions on the speed dial with flexDirection: 'row-reverse'

mbrookes · 2018-07-24T08:47:07Z

Would you like to submit a PR for that?

seanchambo · 2018-07-24T23:57:59Z

Yeah I can add it to this PR if you want

seanchambo · 2018-07-25T01:15:03Z

@mbrookes I've added the commit to allow the actions to be placed in any direction

Stale

mbrookes · 2018-07-25T15:42:52Z

@seanchambo Great!

@oliviertassinari Any thoughts on the prop name for the direction the SpeedDial opens? actionsPlacement is a bit verbose.

mbrookes · 2018-07-25T19:14:50Z

@seanchambo For want of anything better, let's go with direction?: 'up' | 'down' | 'left' | 'right';

It would also be good to add a demo to the docs with perhaps a radio to select the direction. You would also need to change the FAB position according to the selected direction.

oliviertassinari · 2018-07-25T19:25:14Z

@mbrookes actionsPlacement sounds good to me 👍

mbrookes · 2018-07-25T20:53:04Z

@oliviertassinari I'm leaning towards direction...

mbrookes

Review comments are based on the assumption we go with direction, so hold off until we agree on that.

mbrookes · 2018-07-25T19:15:43Z

    display: 'flex',
-    flexDirection: 'column-reverse', // Place the Actions above the FAB.
+  },
+  /* Styles when actions are placed to the top */


Please follow the conventions used for these docs comments in rest of the components:

/* Styles applied to the root and action container elements when direction="up" */

mbrookes · 2018-07-25T20:29:22Z

+  actionsTop: {
+    flexDirection: 'column-reverse',
+  },
+  /* Styles when actions are placed to the bottom */


mbrookes · 2018-07-25T20:29:58Z

+
    return (
-      <div className={classNames(classes.root, classNameProp)} {...other}>
+      <div className={classNames(classes.root, classNameProp, actionsPlacementClass)} {...other}>


classNameProp goes last.

mbrookes · 2018-07-25T20:39:33Z


 SpeedDial.propTypes = {
+  /**
+   * The placement of floating actions buttons in relation to speed dial.


"The direction the actions open relative to the floating action button."

mbrookes · 2018-07-25T20:51:37Z

      onClick,
      open,
      tooltipTitle,
+      tooltipPosition,


tooltipPlacement? (Might as well keep it consistent with the Tooltip prop name).

seanchambo · 2018-07-26T01:03:25Z

There is a problem that I think needs to be addressed before we continue down this path.

The placement of the FAB will not honour the absolute positioning. For instance, if the absolute positioning was the bottom right hand corner and the direction was down. The FAB would be moved up the page and the last action in the actions list would be positioned in the bottom right hand corner. This is because of the structure of the SpeedDial component. This structure would need to be changed in order for the absolute positioning to be placed on the FAB.

I'm not sure if this is the expected behavior but I wanted to point this out before we go ahead.

mbrookes · 2018-07-26T08:10:04Z

@seanchambo Yes:

You would also need to change the FAB position according to the selected direction.

Unless I'm missing something, you wouldn't want to absolute position in the bottom right, and have the actions go down or right.

BTW Lets go with "direction" as the prop name.

oliviertassinari · 2018-07-26T08:13:09Z

BTW Lets go with "direction" as the prop name.

By the way, there is a native property with this name.

mbrookes · 2018-07-26T08:23:59Z

there is a native property with this name

There's the HTML dir attribute, and CSS direction style, but I didn't find a direction attribute when I checked. Am I missing something?

oliviertassinari · 2018-07-26T08:25:32Z

My bad then :)

…

On Thu, Jul 26, 2018, 10:24 Matt Brookes ***@***.***> wrote: there is a native property with this name There's the HTML dir attribute, and CSS direction style, but I didn't find a direction attribute when I checked. Am I missing something? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12244 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADBNwwwBt5IxWCLSUpy1lAZLlmRvMdmkks5uKXy8gaJpZM4VaRNn> .

seanchambo · 2018-07-27T01:34:35Z

@mbrookes Okay great, you are right those scenarios aren't very likely but was making sure that all cases didn't need to be accounted for.
Happy to move forward then and change the prop name to direction

seanchambo · 2018-08-01T06:05:48Z

@mbrookes have updated the PR to take care of your comments

let me know what you think

# Conflicts: # packages/material-ui-lab/src/SpeedDial/SpeedDial.js # pages/lab/api/speed-dial.md

mbrookes

@seanchambo Sorry for the delay, I was working on something else. LGTM.

seanchambo · 2018-08-07T00:40:29Z

No worries, we happy to merge?

mbrookes · 2018-08-07T00:47:52Z

@oliviertassinari any objection?

oliviertassinari · 2018-08-07T08:05:49Z

@mbrookes No objection :)

mbrookes · 2018-08-07T11:41:16Z

@seanchambo Thanks!

During review mui#12244 the names for directions changed and CI did not run which introduced errors.

During review #12244 the names for directions changed and CI did not run which introduced errors.

seanchambo force-pushed the tooltip_placement_speed_dial_action branch 3 times, most recently from 017fcc0 to e45ece3 Compare July 23, 2018 03:48

mbrookes previously approved these changes Jul 23, 2018

View reviewed changes

oliviertassinari added the package: lab Specific to the lab. label Jul 24, 2018

seanchambo force-pushed the tooltip_placement_speed_dial_action branch 2 times, most recently from f4c06c0 to 1236ef1 Compare July 25, 2018 01:25

mbrookes reviewed Jul 25, 2018

View reviewed changes

seanchambo force-pushed the tooltip_placement_speed_dial_action branch from 1236ef1 to d0e59e8 Compare August 1, 2018 05:25

Allow a tooltip placement prop on SpeedDialAction

f970ad4

seanchambo force-pushed the tooltip_placement_speed_dial_action branch 2 times, most recently from a5775fe to 1ee2436 Compare August 1, 2018 05:51

Allows the actions of speed dial to be placed in any direction

1a12449

# Conflicts: # packages/material-ui-lab/src/SpeedDial/SpeedDial.js # pages/lab/api/speed-dial.md

seanchambo force-pushed the tooltip_placement_speed_dial_action branch from 1ee2436 to 1a12449 Compare August 1, 2018 23:51

mbrookes approved these changes Aug 5, 2018

View reviewed changes

mbrookes merged commit d561bce into mui:master Aug 7, 2018

eps1lon mentioned this pull request Aug 15, 2018

[slider] Fix transitions #12531

Merged

eps1lon added a commit to eps1lon/material-ui that referenced this pull request Aug 15, 2018

[SpeedDial] fix invalid prop direction supplied to SpeedDial

8bef369

During review mui#12244 the names for directions changed and CI did not run which introduced errors.

eps1lon mentioned this pull request Aug 15, 2018

[SpeedDial] Fix invalid prop direction supplied #12533

Merged

oliviertassinari pushed a commit that referenced this pull request Aug 15, 2018

[SpeedDial] Fix invalid prop direction supplied (#12533)

57e59af

During review #12244 the names for directions changed and CI did not run which introduced errors.

mbrookes added type: new feature Expand the scope of the product to solve a new problem. scope: speed dial Changes related to the speed dial. labels Sep 13, 2018

snyk-bot mentioned this pull request Sep 30, 2019

[Snyk] Upgrade @material-ui/core from 1.0.0 to 1.5.1 Matthelonianxl/nifty-wallet#7

Open

This was referenced Mar 26, 2020

[Snyk] Upgrade @material-ui/core from 1.0.0 to 1.5.1 AndreiShybin/metamask-extension#9

Merged

[Snyk] Upgrade @material-ui/core from 1.0.0 to 1.5.1 baophucct/metamask-extension#5

Open

oliviertassinari removed the package: lab Specific to the lab. label Jan 9, 2021

oliviertassinari changed the title ~~[SpeedDialAction] Allow a tooltip placement prop~~ [SpeedDial] Allow a tooltip placement prop Jan 9, 2021

Uh oh!

Uh oh!

Conversation

seanchambo commented Jul 23, 2018 • edited by mbrookes Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbrookes commented Jul 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seanchambo commented Jul 24, 2018

Uh oh!

mbrookes commented Jul 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seanchambo commented Jul 24, 2018

Uh oh!

seanchambo commented Jul 25, 2018

Uh oh!

mbrookes commented Jul 25, 2018

Uh oh!

mbrookes commented Jul 25, 2018

Uh oh!

oliviertassinari commented Jul 25, 2018

Uh oh!

mbrookes commented Jul 25, 2018

Uh oh!

mbrookes left a comment

Choose a reason for hiding this comment

Uh oh!

mbrookes Jul 25, 2018

Choose a reason for hiding this comment

Uh oh!

mbrookes Jul 25, 2018

Choose a reason for hiding this comment

Uh oh!

mbrookes Jul 25, 2018

Choose a reason for hiding this comment

Uh oh!

mbrookes Jul 25, 2018

Choose a reason for hiding this comment

Uh oh!

mbrookes Jul 25, 2018

Choose a reason for hiding this comment

Uh oh!

seanchambo commented Jul 26, 2018

Uh oh!

mbrookes commented Jul 26, 2018

Uh oh!

oliviertassinari commented Jul 26, 2018

Uh oh!

mbrookes commented Jul 26, 2018

Uh oh!

oliviertassinari commented Jul 26, 2018 via email

Uh oh!

seanchambo commented Jul 27, 2018

Uh oh!

seanchambo commented Aug 1, 2018

Uh oh!

mbrookes left a comment

Choose a reason for hiding this comment

Uh oh!

seanchambo commented Aug 7, 2018

Uh oh!

mbrookes commented Aug 7, 2018

Uh oh!

oliviertassinari commented Aug 7, 2018

Uh oh!

mbrookes commented Aug 7, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

seanchambo commented Jul 23, 2018 •

edited by mbrookes

Loading

mbrookes commented Jul 23, 2018 •

edited

Loading

mbrookes commented Jul 24, 2018 •

edited

Loading