Skip to content

[SIEM] Filter out "loading" from Panel to make it more React performant - #46258

Merged
FrankHassanabad merged 2 commits into
elastic:masterfrom
FrankHassanabad:use-filtering-for-loading-boolean
Sep 21, 2019
Merged

FrankHassanabad merged 2 commits into
elastic:masterfrom
FrankHassanabad:use-filtering-for-loading-boolean

Conversation

@FrankHassanabad

@FrankHassanabad FrankHassanabad commented Sep 20, 2019 •

Copy link
Copy Markdown
Contributor

Summary

  • This filters out the loading boolean from the EuiPanel to remove React console warnings
  • This makes React memo's more performant by not pushing down new objects potentially causing re-renders.
  • This improves the syntax usage from awkward object to a regular boolean value
  • This DRY (Do not Repeat Yourself) the Panel to put in one area for usage
  • This adds a unit test to ensure loading=true does not end up on the DOM

References:
styled-components/styled-components#1198 (comment)
#41596 (comment)
https://www.styled-components.com/docs/faqs#why-am-i-getting-html-attribute-warnings
https://reactjs.org/blog/2017/09/08/dom-attributes-in-react-16.html

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials

For maintainers

- [ ] This was checked for breaking API changes and was labeled appropriately
- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately

@FrankHassanabad FrankHassanabad self-assigned this Sep 20, 2019
@FrankHassanabad FrankHassanabad added v8.0.0 v7.5.0 release_note:skip Skip the PR/issue when compiling release notes release_note:fix Team:SIEM and removed release_note:skip Skip the PR/issue when compiling release notes labels Sep 20, 2019
@elasticmachine

Copy link
Copy Markdown
Contributor

Pinging @elastic/siem

@FrankHassanabad FrankHassanabad changed the title [SIEM] Filter out "loading" from Panel to make it a more React performant boolean [SIEM] Filter out "loading" from Panel to make it more React performant Sep 20, 2019
@elasticmachine

Copy link
Copy Markdown
Contributor

💔 Build Failed

@FrankHassanabad

Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

@elasticmachine

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@angorayc angorayc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @FrankHassanabad, I'm happy to learn a new thing of styled component. It does make the code cleaner.

@FrankHassanabad
FrankHassanabad merged commit 0ec95a5 into elastic:master Sep 21, 2019
@FrankHassanabad
FrankHassanabad deleted the use-filtering-for-loading-boolean branch September 21, 2019 00:20
FrankHassanabad added a commit to FrankHassanabad/kibana that referenced this pull request Sep 21, 2019
…mant boolean (elastic#46258)

## Summary

* This filters out the loading boolean from the EuiPanel to remove React console warnings
* This makes React memo's more performant by not pushing down new objects potentially causing re-renders.
* This improves the syntax usage from awkward object to a regular boolean value
* This DRY (Do not Repeat Yourself) the Panel to put in one area for usage
* This adds a unit test to ensure loading=true does not end up on the DOM  

References:
styled-components/styled-components#1198 (comment)
elastic#41596 (comment)
https://www.styled-components.com/docs/faqs#why-am-i-getting-html-attribute-warnings
https://reactjs.org/blog/2017/09/08/dom-attributes-in-react-16.html
### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~
~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~
~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~
- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~

### For maintainers

~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~
~~- [ ] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~
FrankHassanabad added a commit that referenced this pull request Sep 22, 2019
…mant boolean (#46258) (#46303)

## Summary

* This filters out the loading boolean from the EuiPanel to remove React console warnings
* This makes React memo's more performant by not pushing down new objects potentially causing re-renders.
* This improves the syntax usage from awkward object to a regular boolean value
* This DRY (Do not Repeat Yourself) the Panel to put in one area for usage
* This adds a unit test to ensure loading=true does not end up on the DOM  

References:
styled-components/styled-components#1198 (comment)
#41596 (comment)
https://www.styled-components.com/docs/faqs#why-am-i-getting-html-attribute-warnings
https://reactjs.org/blog/2017/09/08/dom-attributes-in-react-16.html
### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~
~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~
~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~
- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~

### For maintainers

~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~
~~- [ ] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
…mant boolean (elastic#46258)

## Summary

* This filters out the loading boolean from the EuiPanel to remove React console warnings
* This makes React memo's more performant by not pushing down new objects potentially causing re-renders.
* This improves the syntax usage from awkward object to a regular boolean value
* This DRY (Do not Repeat Yourself) the Panel to put in one area for usage
* This adds a unit test to ensure loading=true does not end up on the DOM  

References:
styled-components/styled-components#1198 (comment)
elastic#41596 (comment)
https://www.styled-components.com/docs/faqs#why-am-i-getting-html-attribute-warnings
https://reactjs.org/blog/2017/09/08/dom-attributes-in-react-16.html
### Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~
~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~
~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~
- [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~

### For maintainers

~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~
~~- [ ] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~
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.

3 participants