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 branch] Compressed form rows #2181

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Jul 30, 2019

feature/compressed-editor-controls

Better spacing and alignment for EuiFormRow

1. Reduces padding, margins, etc when EuiFormRow is compressed

For both compressed and non-compressed, I reduced the spacing between the form control and the help text below to be the same amount used for the form label. This condenses that space a bit but creates a clearer connection between the elements.

2. Adds display="compressedHorizontal" option for EuiFormRow

To allow laying out the label and control on the same line.

Screen Shot 2019-07-30 at 18 14 17 PM

I applied this as an option to the compressed prop because it should really only be used in conjunction with compressed controls. And while we can't know for sure if they're also passing a compressed control as the child, making this an option under the prop name "compressed" may deter people from using it outside of a compressed form.

3. Removed EuiFlexGroup dependency for label appends

And instead just writing out the flex styles directly. This also allowed for better calculation of spacing.

Breaking

There are several "breaks" in this PR:

  1. EuiFormRow no longer has bottom padding, nor does it add margin to any + * siblings. Instead, it only adds top margin to siblings of the same type .euiFormRow + .euiFormRow This means if layouts relied on margin being added to non-form row children, they'll need spacers added.

So you'll see a lot of <EuiSpacer /> additions to files. I think this is a better approach as you never truly know what that universal selector will be attaching margins to. However, if someone feels strongly that we should not introduce a break like this, I should be able to easily revert.

  1. EuiFormRow no longer passes compressed down to its children. This was not correct anyway but React wasn't complaining because the prop was fully lowercase. If the child didn't have a prop called compressed it was being passed down as an html attribute like <input compressed /> which is invalid.

I finally removed this because it was causing an error when I added the third option of horizontal.

  1. Removed bottom margin from EuiFormLabel, but added it back to the wrapping element inside of the EuiFormRow so there's still margin but just not on the plain element.

This means if anyone used EuiFormLabel by itself and relied on that margin, they'd need to add a spacer.

Deprecations

  1. EuiFormRow's compressed prop is now set for deprecation in favor of display = "compressed".
  2. EuiFormRow's displayOnly prop is now set for deprecation in favor of display = "center".

Docs

I'll be reverting all but the Compressed and horizontal section of the form layout docs

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 Will be adde via the feature branch

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.

These are my big questions. I haven't done a full code review yet but wanted to get you something to chew on since I'm tardy on this. Perfectly OK to do some pair / idea shaking on this over zoom or something too.

src/components/form/form_row/_form_row.scss Show resolved Hide resolved
src/components/form/form_row/form_row.js Outdated Show resolved Hide resolved
src-docs/src/views/form_layouts/form_rows.js Outdated Show resolved Hide resolved
src/global_styling/mixins/_typography.scss Show resolved Hide resolved
@cchaos cchaos added the deprecations Contains deprecations. Add them to the deprecations meta ticket after merge. label Aug 8, 2019
@cchaos cchaos requested a review from snide August 9, 2019 02:47
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.

Small comments at this point. I'll leave it to you to figure out the naming you think is best. Also assume you'll revert the doc layer.

I'm a little worried about the Spacer solution, but I think when we finally release this, we should probably assign each other apps and make everyone hand check a lot of the forms. My guess is we'll need to spot fix some of those usages.

I ran through the code and did a pretty thorough docs scan.

src/components/form/form_row/form_row.js Show resolved Hide resolved
src/components/form/form_row/form_row.js Show resolved Hide resolved
src-docs/src/views/form_layouts/form_layouts_example.js Outdated Show resolved Hide resolved
src/components/form/form_row/index.d.ts Outdated Show resolved Hide resolved
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.

I'm good with the sibling spacing and EuiSpacer decisions. The prop enums you landed on make sense, also.

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.

👍 Looks and works as designed (and as tested in Sketch test). Can't wait to use it!

This reverts commit cd71516.

# Conflicts:
#	src-docs/src/views/form_layouts/inline_sizing.js
@cchaos cchaos merged commit 62dc3aa into elastic:feature/compressed-editor-controls Aug 13, 2019
@cchaos cchaos deleted the into-feature/compressed-form-rows branch August 13, 2019 13:29
cchaos added a commit that referenced this pull request Aug 13, 2019
* 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`
cchaos added a commit that referenced this pull request Aug 29, 2019
* 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`
cchaos added a commit that referenced this pull request Aug 30, 2019
* 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`
@snide snide mentioned this pull request Sep 5, 2019
13 tasks
@cchaos cchaos mentioned this pull request Sep 6, 2019
9 tasks
cchaos added a commit that referenced this pull request Sep 9, 2019
* 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`
thompsongl pushed a commit to thompsongl/eui that referenced this pull request Sep 10, 2019
* 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`
cchaos added a commit that referenced this pull request Sep 11, 2019
* [Feature branch] Updated form control border color (#2114)

* Updated form control border color

* Slighly more transparent

* change sass var name to $euiFormBorderOpaqueColor

* [Feature branch]  Added EuiFormControlLayoutDelimited component (#2117)

As a layout helper component to create date and number ranges

* Added Sass var for `$euiFormControlLayoutGroupInputHeight` and compressed version

* [Feature branch] Compressed EuiSuperSelect dropdown (#2155)

- Added truncation example
- Added max-height

* [Feature Branch] Update compressed form control styles (#2174)

* 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

* [Feature branch] Compressed form rows (#2181)

* 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`

* Fix snap

* Into feature/compressed editor docs (#2295)

* 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

* [Feature Branch] Add support for EuiRange in a dropdown with input (#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

* Added some display toggles for ranges and ranges with dropdowns to the complex example

Has issues

* Fix console errors

* Some fixes

- Ranges use div spacers between slider and input instead of margin
- Pre/Appends are restrictred to 50% width and truncated

* [feature/compressed-editor-controls] EuiRange + EuiFormRow (#2321)

* focus management

* add euiformrow to some examples

* use prevention flag

* Add `controlOnly` prop to EuiFieldText

* Update TS defs

* Some docs fixes

* Fix inherit screen reader ability of EuiRange

By moving the id to the input if it exists

* Add dual range to complex example
@snide snide mentioned this pull request Sep 16, 2019
35 tasks
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