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

Adds possibility to disable checkboxes individually in an EuiCheckboxGroup component #2548

Conversation

ffknob
Copy link
Contributor

@ffknob ffknob commented Nov 19, 2019

Summary

Related to #2539

Added disabled option to the option attribute of the options object that is passed to the EuiCheckboxGroup so that checkboxes in a group can be individually disabled

Checklist

  • Checked in dark mode
  • Checked in mobile
  • Checked in IE11 and Firefox
  • Props have proper autodocs
  • Added documentation examples
  • Added or updated jest tests
  • Checked for breaking changes and labeled appropriately
  • Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

…at is passed to the EuiCheckboxGroup so that checkboxes in a group can be individually disabled
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@ffknob ffknob changed the title EuiCheckboxGroup: Adds disabled option to the option attribute of the options object Adds possibility to disable checkboxes individually in an EuiCheckboxGroup component Nov 19, 2019
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Looks good, just needs a PropChange & TypeScript change for the disabled value.

@@ -21,7 +21,7 @@ export const EuiCheckboxGroup = ({
id={option.id}
checked={idToSelectedMap[option.id]}
label={option.label}
disabled={disabled}
disabled={disabled || option.disabled}
Copy link
Contributor

Choose a reason for hiding this comment

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

disabled needs to be added to this component's propTypes.options shape definition (lines 34-39). Similarly, the typescript definition needs to be updated -

export interface EuiCheckboxGroupOption {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@@ -56,4 +56,22 @@ describe('EuiCheckboxGroup (mocked checkbox)', () => {

expect(component).toMatchSnapshot();
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the test!

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Changes LGTM; pulled & tested locally. Will merge after passing Jenkins/CI run

@chandlerprall
Copy link
Contributor

jenkins test this

@chandlerprall
Copy link
Contributor

Thanks again @ffknob !

@ffknob
Copy link
Contributor Author

ffknob commented Nov 20, 2019

Thanks again @ffknob !

Glad to help!

@chandlerprall chandlerprall merged commit 079e071 into elastic:master Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants