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

Disable "set as default" menu item for the default index set #3385

Merged
merged 1 commit into from Jan 18, 2017
Merged

Conversation

@bernd
Copy link
Member

@bernd bernd commented Jan 18, 2017

Fixes #3383

@bernd bernd added this to the 2.2.0 milestone Jan 18, 2017
@bernd bernd requested a review from edmundoa Jan 18, 2017
@edmundoa edmundoa self-assigned this Jan 18, 2017
Copy link
Member

@edmundoa edmundoa left a comment

Apart from the inline comment, these changes still do not affect my default index set, which still has the option and I can click on it.

const menuItems = [];

if (indexSet.writable) {
menuItems.push(<MenuItem onSelect={this._onSetDefault(indexSet)} disabled={!indexSet.writable}>Set as default</MenuItem>);

This comment has been minimized.

@edmundoa

edmundoa Jan 18, 2017
Member

With this change the menu item would never be disabled, this line of code would not be reached for non-writable index sets.

By the way, I like better the idea of having some title saying why the item is disabled instead of just removing it from the menu. It would make it more clear imho.

This comment has been minimized.

@bernd

bernd Jan 18, 2017
Author Member

I didn't read the issue careful enough and removed the menu item when not writable. I will change it to only disable it in that case.

@bernd bernd force-pushed the issue-3383 branch from db8bcbf to 0728fef Jan 18, 2017
@bernd bernd changed the title Do not show the "set as default" action for non-writable index sets Disable "set as default" menu item for the default index Jan 18, 2017
@bernd bernd changed the title Disable "set as default" menu item for the default index Disable "set as default" menu item for the default index set Jan 18, 2017
Copy link
Member

@edmundoa edmundoa left a comment

LGTM 👍

@edmundoa edmundoa merged commit 78c4185 into master Jan 18, 2017
2 of 4 checks passed
2 of 4 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
@garybot2
ci-web-linter Jenkins build graylog-pr-linter-check 1300 has succeeded
Details
licence/cla Contributor License Agreement is signed.
Details
@edmundoa edmundoa deleted the issue-3383 branch Jan 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants