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

[Feature] Compressed editor controls #2167

Merged
merged 22 commits into from
Sep 11, 2019
Merged

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Jul 25, 2019

Created better compressed form controls

This PR has accumulated in the creation of:

Screen Shot 2019-09-10 at 11 10 58 PM

💝 Highlights

🔜 Todos

What's still left to do has been added to this meta ticket #2286

💔 Breaks

EuiFormRow #2181

  • No longer passes compressed to it's children
  • No longer has bottom padding, nor does it add margin to any + * siblings.

EuiFormLabel #2181

  • Removed bottom margin

Removed SCSS mixin euiTextOverflowWrap() and created euiTextBreakWord() instead as the latter works more appropriately.

⚠️ Deprecations

EuiFormRow #2181

  • compressed prop is now set for deprecation in favor of display = "compressed"
  • displayOnly prop is now set for deprecation in favor of display = "center"

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

@cchaos cchaos force-pushed the feature/compressed-editor-controls branch from 836497d to 880ea3f Compare July 29, 2019 19:20
@cchaos cchaos force-pushed the feature/compressed-editor-controls branch from 62dc3aa to 689d79b Compare August 13, 2019 13:30
@cchaos cchaos force-pushed the feature/compressed-editor-controls branch 2 times, most recently from 8d3ae71 to eb61262 Compare August 30, 2019 17:15
@snide snide mentioned this pull request Sep 5, 2019
13 tasks
@cchaos cchaos added the deprecations Contains deprecations. Add them to the deprecations meta ticket after merge. label Sep 6, 2019
cchaos and others added 8 commits September 9, 2019 18:42
* Updated form control border color

* Slighly more transparent

* change sass var name to $euiFormBorderOpaqueColor
As a layout helper component to create date and number ranges

* Added Sass var for `$euiFormControlLayoutGroupInputHeight` and compressed version
- Added truncation example
- Added max-height
* Updated compressed visual style in mixin
* Compressed updates to from control groups
* Fix compressed state overrides
* Reduce horizontal padding for compressed
* Icons and button icons in input groups
* Added a compressed option for from` euiFormControlLayoutPadding`
* Added compressed padding for inputs with icons
* Fix readonly & compressed input groups
* Fix group heights
* Update file picker with new compressed styles
* Fix delimited compressed and fullwidth styles
* Fixed EuiComboBox
* Added reduced padding for EuiColorPicker
* Fixed date pickers
* Variables for border-radius
* Removed padding from compressed form row
* Create mixin for `euiTextBreakWord`
* Added option for horizontal compressed style

Breaking: `compressed` is no longer passed to children

* [Docs] Final compressed doc example changes
* Fix combobox height
* Fixed usages where spacers were needed
* Deprecated `displayOnly` for `display: center`
* Adding a shared component for the different states of each form control
* New docs section for compressed
* Added `columnCompressedSwitch` to display type of EuiFormRow for better alignment of switch style controls
* Account for tooltips as pre/appends
* Allow passing a string as the pre/appends and it automatically wraps it in a form label
* Made all EuiSuperDatePicker form controls compressed (in popover)
* Connection prepend/appends with input via `htmlFor`
* Allowing passing of `style` to EuiColorPickerSwatch
* Allow an array of strings a pre/append
…2179)

* Compressed EuiRange step 1: Decrease overall height of the range when compressed
* Compressed EuiRange step 2:
- Attempt at single range compressed input with popover
- Fixes z-indexes being too high
- Fix up widths
* Compressed EuiRange step 3: Dual range now supports dropdown style
* Fix for delimited
* Fix full-width delimited
* Added `controlOnly` prop to EuiFieldNumber
* Finalize styles of input only ranges
- Needed some fixes to EuiFormControlLayoutDelimited
* Open popover on focus
* dual range respond to resize events when in showInputOnly mode
* use callback instead of resizeObserver
* ie11 focus fix
* use focusout instead of blur
@cchaos cchaos force-pushed the feature/compressed-editor-controls branch from 7f88db7 to 9ee5620 Compare September 9, 2019 22:42
cchaos and others added 8 commits September 10, 2019 15:47
- Ranges use div spacers between slider and input instead of margin
- Pre/Appends are restrictred to 50% width and truncated
* focus management

* add euiformrow to some examples

* use prevention flag

* Revert "add euiformrow to some examples"

This reverts commit f93ef4e.

* clean up
…om/elastic/eui into feature/compressed-editor-controls

# Conflicts:
#	src/components/form/range/dual_range.js
@cchaos cchaos marked this pull request as ready for review September 11, 2019 02:59
@cchaos
Copy link
Contributor Author

cchaos commented Sep 11, 2019

This PR is now ready for review. I just have to do the final checklist including a long changelog entry. For those tagged as reviewers, these changes have all been "reviewed" already via the earlier PR's so feel free to do as much review as you think necessary. I did a once over on all the files and the docs - fixed a few issues found. I'd just like a final 👍 from each of you before we deem it mergeable.

🙇‍♀ Thank you!!

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

I reviewed most of this stuff first time around so I'm just concentrating on the docs. One thing I noticed for accessibility is that the range controls / steppers don't seem to properly announce the aria-labels used. This looks unchanged from master, so I don't think it's a blocker, but is easy to replicate if you want to give it a check. Just tab into any of the range controls on your compressed form examples (the big panel ones).

I did a visual scan of every EUI page. Here are some small breaks I noticed

Modals

image

Combo box loading state

image

Invalid states on readonly / disabled

I think this makes sense, but just calling it out, that readonly / disabled negates the invalid state. Again, I think this makes sense, just making sure.

Expressions

Might want to update these examples to use compressed controls.

image

@cchaos
Copy link
Contributor Author

cchaos commented Sep 11, 2019

Thanks @snide !

Combo box loading state

Apparently the loading state only affects the options dropdown.
Screen Shot 2019-09-11 at 10 57 20 AM

Invalid states on readonly / disabled

I don't think I changed this from before but I too think it makes sense since you wouldn't be able to "fix" the error state.


The others have been done.

Screen Shot 2019-09-11 at 11 05 20 AM

Screen Shot 2019-09-11 at 11 05 03 AM

I'll look into the screen reader issue for ranges.

By moving the id to the input if it exists
@cchaos
Copy link
Contributor Author

cchaos commented Sep 11, 2019

Ok, I was able to fix the EuiRange screen reader a11y, but the EuiDualRange will need a lot of work that I'd like to push off to another PR. I'll start an issue.

Screen Shot 2019-09-11 at 11 51 47 AM

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

We reviewed everything in here, so there should be no surprises. I think we just need to anticipate there will be some patches needed post merge / attempting to get this into Kibana.

Once merged, let's send an email out to dev and give some guidance. Before we do that, let's get some signoff from @chandlerprall and @thompsongl that we don't have anything time sensitive to get into EUI before this lands (to minimize a need for a backport).

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

@snide I'm not aware of anything important that needs to get in before this.

Do we still want to cut a release before this gets in? There could be a couple bugfixes that merge today (along with one already on master)

@cchaos
Copy link
Contributor Author

cchaos commented Sep 11, 2019

I think we just need to anticipate there will be some patches needed post merge / attempting to get this into Kibana.

Absolutely, there will be bugs found and features needed. But this is a good start.

Do we still want to cut a release before this gets in?

Yes. Right before this is ready to merge, we should do a release then merge, then release again.

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

This is looking quite solid and seems ready to merge. Let's get it out in the wild and work out any other kinks. Thanks Caroline!

@cchaos cchaos merged commit ab29a11 into master Sep 11, 2019
@cchaos cchaos mentioned this pull request Sep 11, 2019
5 tasks
@cchaos cchaos deleted the feature/compressed-editor-controls branch October 22, 2019 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change deprecations Contains deprecations. Add them to the deprecations meta ticket after merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants