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.

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

COMPASS-463 action buttons disable secondary #717

Merged
merged 11 commits into from Jan 5, 2017

Conversation

satyasinha
Copy link

No description provided.

@satyasinha satyasinha force-pushed the COMPASS-463-action-buttons-disable-secondary branch from dc4360c to 0bb394d Compare December 23, 2016 05:12
@rueckstiess
Copy link
Member

Still needs some kind of tooltips / message so that the user knows why the button is disabled.

@satyasinha
Copy link
Author

Should this be a react-tooltip? Or a 2 second toastie/error dialog possibly similar to RTSS errors?

Or something fancier that the design team can help with?

On the other hand once https://jira.mongodb.org/browse/COMPASS-288 is done a tooltip/message might be somewhat redundant as the replica set status would be mentioned there.

@rueckstiess
Copy link
Member

See comments on COMPASS-463: black react-tooltip. Perhaps they can go to the side?

Language something like: "Cannot <do action> on a secondary node." (replace <do action> with whatever the button does).

@satyasinha
Copy link
Author

satyasinha commented Jan 3, 2017

screen shot 2017-01-03 at 5 16 14 pm

something like this? @rueckstiess @fredtruman

edit: I might change the text to 'This action is not available on a secondary node'
The capitals seem a little aggressive.

@rueckstiess
Copy link
Member

Can you try how these changes look:

  • no transparency
  • no border
  • single line
  • to the right side
  • and yes I think your wording is better

@fredtruman
Copy link
Contributor

What Thomas said 😄

@satyasinha satyasinha force-pushed the COMPASS-463-action-buttons-disable-secondary branch from 0bb394d to a559e8d Compare January 4, 2017 03:43
@satyasinha
Copy link
Author

I've pushed a commit but the tooltips don't appear well next to buttons, I've had to add 'data-offset': '{"left": 900}' parameter, which doesn't really work if the window is resized. Ideally it would be nice if the 'data-offset' parameter isn't required at all. Maybe some kind of css positioning to align it on the right side of the button?

I've added a new css file to work with react-tooltip for these tooltips here: https://github.com/10gen/compass/blob/a559e8d5c01c49ca39336ac4d5791dc885714b2f/src/internal-packages/app/styles/secondary-tooltip.less

If one of you could please look into this? @fredtruman @Sean-Oh

@rueckstiess
Copy link
Member

rueckstiess commented Jan 4, 2017

Made the following changes:

  • Only one global <ReactTooltip /> component necessary
  • Disabled buttons have pointer events disabled and thus have to be wrapped in an inline-block div and the attributes go on the wrapper div
  • slight change in style and renamed to not-writable-tooltip.less because of the confusion of a "secondary tooltip" vs. a "primary tooltip"

@rueckstiess
Copy link
Member

screen shot 2017-01-04 at 16 55 01

@rueckstiess rueckstiess changed the title Compass 463 action buttons disable secondary COMPASS-463 action buttons disable secondary Jan 4, 2017
@fredtruman
Copy link
Contributor

These latest tooltips LGTM 👍

@Sean-Oh
Copy link
Contributor

Sean-Oh commented Jan 4, 2017

LGTM as well! 👍

@rueckstiess rueckstiess self-requested a review January 5, 2017 00:46
Copy link
Member

@rueckstiess rueckstiess left a comment

Choose a reason for hiding this comment

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

Needs rebase, then good to go. I'll do that quickly.

@rueckstiess rueckstiess force-pushed the COMPASS-463-action-buttons-disable-secondary branch from 265616a to 1270d79 Compare January 5, 2017 01:00
@rueckstiess rueckstiess merged commit 3aacbec into master Jan 5, 2017
@rueckstiess rueckstiess deleted the COMPASS-463-action-buttons-disable-secondary branch January 5, 2017 02:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants