Chore/decouple button content #3730

cchaos · 2020-07-13T18:55:15Z

Pre-requisite to finishing #3099

EuiButtonGroup will not be able to use <EuiButton> directly, and therefore, we will need to rebuild a lot of styles so that they can look/accept the same params as EuiButton, but render as a <label>.

Instead of just allowing EuiButton to render as a <label> to anyone (since this can easily be abused), there are 2 pre-requisites to make it easier to re-apply EuiButton content and styles that this PR does.

EuiButtonContent

This is a new internal-only component that just pulls out the spans creating the content span that encompasses the icon and text span. Now used by both EuiButton and EuiButtonEmpty (possibly others in the future).

EuiButtonDisplay

Also an inernal-only component, and truly an internal to EuiButton only component as well. This is basically all of the original code for EuiButton but with the additional prop of element which can be span, a, button, label, etc...

Then the EuiButton code itself is just the a vs button that it passes to <EuiButtonDisplay>.

Checklist

Check against all themes for compatibility in both light and dark modes
[ ] Checked in mobile
[ ] Checked in IE11 and Firefox
[ ] Props have proper autodocs
[ ] Added documentation
~~[ ] Checked Code Sandbox works for the any docs 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

cchaos

@chandlerprall or @thompsongl I'd love your help on this one for TS. I've outlined my issues below.

src/components/button/button.tsx

src/components/button/button_content.test.tsx

thompsongl · 2020-07-13T19:04:24Z

I'd love your help on this one for TS

I'll take a look

thompsongl · 2020-07-13T20:35:24Z

👉 cchaos#34

kibanamachine · 2020-07-14T16:23:44Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_3730/

src/components/button/button.tsx

For rendering the contents of buttons, icon (loading spinner) and text wrapper for children

cchaos · 2020-07-14T17:51:15Z

Actually hold off a bit on reviewing, I've found a couple extra places for optimization

kibanamachine · 2020-07-14T17:57:18Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_3730/

- Extend EuiButtonContentProps - Content styles are in button_content.scss

cchaos · 2020-07-14T20:39:49Z

Ok, now this is ready for review. A lot of the changed files are just snapshots since this PR changes the classNames of some of the button internals.

kibanamachine · 2020-07-14T21:01:04Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_3730/

thompsongl

Overall the changes LGTM.
After seeing the comment from @myasonik about a more curated list of internal elements, it may be worth adding a code comment that links to this PR so we can follow the thread to the source if necessary.

cchaos · 2020-07-15T15:35:17Z

Alright, I'll give in and create basically an allow list for element. 😉 I don't think we should/need to link to PR's in code comments since it should be easily found via git-blame.

elizabetdev

LGTM! 🎉

I looked at the code and tested the EuiButton and EuiButtonEmpty to see if they work as expected. I couldn't find any issue.

kibanamachine · 2020-07-15T17:05:28Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_3730/

* Break up release.js (elastic#3723) * Switch release.js to use arguments instead of env vars * Switch MFA code back to env var so it doesn't leak in CI logs * Update job definition to use --type arg * Support breaking up release steps with args * Break release up to fetch time-sensitive MFA token right before publish * Strip whitespace from each step Co-authored-by: Chandler Prall <[email protected]> Co-authored-by: Chandler Prall <[email protected]> * Allow prop-setting on the FlexItems within DescribedFormGroup (elastic#3717) * Allow prop-setting on the FlexItems within DescribedFormGroup * Add changelog entry * Honor classes on fieldFlexItem * Update src/components/form/described_form_group/described_form_group.test.tsx Co-authored-by: Caroline Horn <[email protected]> * Update snap name Co-authored-by: Caroline Horn <[email protected]> * Re-focus EuiSuperSelect input after making a value change (elastic#3734) * Re-focus EuiSuperSelect input after making a value change * changelog * [EuiStat] Allow customizing the render of the title and description HTML tags (elastic#3693) * Add titleElement and descriptionElement to EuiStat * Updated snapshots * Updated changelog * Fixed issue with screenreader * Updated snapshots * Use screen reader only if title and description is a string * updated snapshots * Merge remote-tracking branch 'upstream/master' into fix/stat * Fixed typo in changelod * removed titleChildren * titlechildren as variable * Update CHANGELOG.md Remove an extra line left over from a merge resolution Co-authored-by: Chandler Prall <[email protected]> * [EuiTable] Expand item action name to allow a function (elastic#3739) * allow for item -> ReactNode in name * docs * CL * Add ssh keys so new tags can be pushed to Github from Jenkins (elastic#3740) * Add ssh keys so new tags can be pushed to Github * Need a vault token before we can pull secrets * update i18ntokens * 27.1.0 * Updated documentation. * Chore/decouple button content (elastic#3730) * [New] EuiButtonContent For rendering the contents of buttons, icon (loading spinner) and text wrapper for children * Use EuiButtonContent in EuiButtonEmpty * Fixing classNames and disabled states * Create EuiButtonDisplay for internal usage * Fix snaps * ts gymnastics * Added tests for EuiButtonContent * More optimization - Extend EuiButtonContentProps - Content styles are in button_content.scss * Restricted list of `element`s * [Docs] Adding more acccessibility-focused notes and examples (elastic#3714) * making more a11y callouts in docs * Apply suggestions from code review Co-authored-by: gchaps <[email protected]> * prettier update Co-authored-by: gchaps <[email protected]> * [EuiFocusTrap] Use `react-focus-on` (elastic#3631) * WIP: react-focus-on * WIP: fix GuideFullScreen * noIsolation; onClickOutside * euiflyout snapshot updates * euiflyout snapshot updates * euimodal snapshot updates * euidatagrid snapshot updates * euicolorpalettepicker snapshot updates * euisuperselect snapshot updates * euicollapsible snapshot updates * euifocustrap snapshot updates * allow for array snapshots with takeMountedSnapshot * docs improvements * default to noIsolation=true and scrollLock=false * CL * Fixed bug in EuiComboBox always showing a scrollbar (elastic#3744) * Fixed EuiComboBox always showing a scrollbar * Adding the right PR number to CL * Added useEuiI18n hook (elastic#3749) * Added useEuiI18n hook * Updated docs with useEuiI18n hook, added snippets * Add support to fetch-1i8n-strings or useEuiI18n to match EuiI18n extraction * Fix up return types for useEuiI18n * Updated custom eslint i18n rule/package to lint useEuiI18n usages * changelog * Remove something I was testing with and lost where I had placeed it. * [EuiSuperDatePicker] Bypass formatting `null` dates (elastic#3750) * prevent formatting on null value * remove unnecessary cast * account for keyboard nav with null selection * CL * Updated EuiComboBox to allow the options list to open for single selection custom options (elastic#3706) * Fixing includes to return true when object exists in array * changelog * Allowing list to open for single selection custom options * Updated changelog * PR review * Improving example * Improving example * Addind isClearable * Improving examples * Improving explanation text * Adding note * Addressing PR issues * Update src-docs/src/views/combo_box/combo_box_example.js Co-authored-by: Caroline Horn <[email protected]> * Update src-docs/src/views/combo_box/combo_box_example.js Co-authored-by: Caroline Horn <[email protected]> * Snippet Co-authored-by: Caroline Horn <[email protected]> * Add analyze event glyph to EuiIcon (elastic#3729) * adding analyze event security icon * updating analyze_event icon * updating again * Update CHANGELOG.md * Update consuming.md (elastic#3769) * Path has changed and the wiki has not been updated. * Fix zIndex for two popup ups (elastic#3768) Clicking both buttons on https://eui.elastic.co/#/tabular-content/data-grid-styling-and-toolbar demo brings up partially hidden popups because their z-index is too low. Increasing by one seems to do the trick. * [Playground] EuiBadge, EuiNotificationBadge, EuiBetaBadge (elastic#3722) * created playground for badges * removed commented code * used validator for iconType and colour * updated variable name * removed colour validation * removed unnecessary imports * removed default values, fullscreen mode * suggesstions * removed placeholder, added required, some badge props set to string * used actual value of child in text field * added sanitize function * fixed unique-key warning * added validation * updated to identify the change whenthe state changes * suggestions * added onCLick function snippet * removed error popup by react-view * removed lint err * removed commented code Co-authored-by: Josh Mock <[email protected]> Co-authored-by: Chandler Prall <[email protected]> Co-authored-by: Scott Gould <[email protected]> Co-authored-by: Caroline Horn <[email protected]> Co-authored-by: Ashik Meerankutty <[email protected]> Co-authored-by: Greg Thompson <[email protected]> Co-authored-by: Michail Yasonik <[email protected]> Co-authored-by: gchaps <[email protected]> Co-authored-by: Elizabet Oliveira <[email protected]> Co-authored-by: Marra Sherrier <[email protected]> Co-authored-by: Alberto Andújar <[email protected]> Co-authored-by: Yuri Astrakhan <[email protected]>

* [New] EuiButtonContent For rendering the contents of buttons, icon (loading spinner) and text wrapper for children * Use EuiButtonContent in EuiButtonEmpty * Fixing classNames and disabled states * Create EuiButtonDisplay for internal usage * Fix snaps * ts gymnastics * Added tests for EuiButtonContent * More optimization - Extend EuiButtonContentProps - Content styles are in button_content.scss * Restricted list of `element`s

This reverts commit 0a9cb7a.

cchaos commented Jul 13, 2020

View reviewed changes

src/components/button/button.tsx Outdated Show resolved Hide resolved

src/components/button/button.tsx Outdated Show resolved Hide resolved

src/components/button/button_content.test.tsx Show resolved Hide resolved

myasonik reviewed Jul 14, 2020

View reviewed changes

src/components/button/button.tsx Outdated Show resolved Hide resolved

cchaos and others added 11 commits July 14, 2020 13:30

[New] EuiButtonContent

be07c13

For rendering the contents of buttons, icon (loading spinner) and text wrapper for children

Use EuiButtonContent in EuiButtonEmpty

b8ff8d7

Fixing classNames and disabled states

84585b9

Create EuiButtonDisplay for internal usage

2a45dca

Fix snaps

cf41154

ts gymnastics

ef8b023

clean up

9ffe430

generic display comopnent

3a718ea

createElement; comments

d109ab1

Added tests for EuiButtonContent

1f2357b

cl

a922012

cchaos force-pushed the chore/decouple_button_content branch from d9b751e to a922012 Compare July 14, 2020 17:32

cchaos marked this pull request as ready for review July 14, 2020 17:33

cchaos requested review from thompsongl and elizabetdev July 14, 2020 17:33

cchaos marked this pull request as draft July 14, 2020 17:51

cchaos and others added 2 commits July 14, 2020 16:37

More optimization

84b61d2

- Extend EuiButtonContentProps - Content styles are in button_content.scss

Merge branch 'master' into chore/decouple_button_content

8c5a5ab

cchaos marked this pull request as ready for review July 14, 2020 20:39

thompsongl approved these changes Jul 14, 2020

View reviewed changes

Restricted list of elements

2b6da39

elizabetdev approved these changes Jul 15, 2020

View reviewed changes

cchaos merged commit b02b840 into elastic:master Jul 15, 2020

cchaos deleted the chore/decouple_button_content branch July 15, 2020 17:22

anishagg17 added a commit to anishagg17/eui that referenced this pull request Jul 20, 2020

Revert "Chore/decouple button content (elastic#3730)"

3bb8e4e

This reverts commit 0a9cb7a.

anishagg17 added a commit to anishagg17/eui that referenced this pull request Jul 20, 2020

Revert "Chore/decouple button content (elastic#3730)"

6b50c32

This reverts commit 0a9cb7a.

thompsongl mentioned this pull request Jul 23, 2020

Upgrade EUI to v27.2.0 elastic/kibana#72993

Closed

thompsongl mentioned this pull request Jul 31, 2020

Upgrade EUI to v27.4.0 elastic/kibana#74004

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore/decouple button content #3730

Chore/decouple button content #3730

cchaos commented Jul 13, 2020 •

edited

Loading

cchaos left a comment

thompsongl commented Jul 13, 2020

thompsongl commented Jul 13, 2020

kibanamachine commented Jul 14, 2020

cchaos commented Jul 14, 2020

kibanamachine commented Jul 14, 2020

cchaos commented Jul 14, 2020

kibanamachine commented Jul 14, 2020

thompsongl left a comment •

edited

Loading

cchaos commented Jul 15, 2020

elizabetdev left a comment •

edited

Loading

kibanamachine commented Jul 15, 2020

Chore/decouple button content #3730

Chore/decouple button content #3730

Conversation

cchaos commented Jul 13, 2020 • edited Loading

Pre-requisite to finishing #3099

Checklist

cchaos left a comment

Choose a reason for hiding this comment

thompsongl commented Jul 13, 2020

thompsongl commented Jul 13, 2020

kibanamachine commented Jul 14, 2020

cchaos commented Jul 14, 2020

kibanamachine commented Jul 14, 2020

cchaos commented Jul 14, 2020

kibanamachine commented Jul 14, 2020

thompsongl left a comment • edited Loading

Choose a reason for hiding this comment

cchaos commented Jul 15, 2020

elizabetdev left a comment • edited Loading

Choose a reason for hiding this comment

kibanamachine commented Jul 15, 2020

cchaos commented Jul 13, 2020 •

edited

Loading

thompsongl left a comment •

edited

Loading

elizabetdev left a comment •

edited

Loading