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] Update compressed form control styles #2174

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Jul 26, 2019

feature/compressed-editor-controls

Primarily updating the existing --compressed styles to match the new style

I also fixed a few styles for the form control layouts as well as allow for more types of elements to be passed via append/prepend.

Screen Shot 2019-07-26 at 16 07 45 PM

Docs

My last commit is [TEMP], it adds compressed to all the examples in Forms/Controls so it's easier to test and evaluate these styles. I'll revert that commit before margins.

Components

I'll go through EUI's usages of any compressed controls in a separate PR.

Breaking!

But in style only. Now compressed=true is a completely different style for forms.

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 get added in the feature branch

@@ -68,9 +72,12 @@ export default class extends Component {

<EuiSelect
options={this.options}
value={this.state.value}
// value={this.state.value}
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not, but I'll be reverting all those docs pages changes before merging. I was just giving you all an easy way to view all the compressed styles.

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.

JS/TSX changes LGTM - I'm loving this progress! Pulled locally and saw pretty things.

@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 into-feature/compressed-styles branch from 5eab41d to 0536eee Compare July 29, 2019 19:20
@snide
Copy link
Contributor

snide commented Jul 30, 2019

Quick one while I'm working through this. I could see this as being difficult if used without regular form rows? Will this need to manually be done?

Saw your note about docs, so dunno if this stuff was just temp.

image

@snide
Copy link
Contributor

snide commented Jul 30, 2019

image

- Spacing around pills
- Compressed version

# Conflicts:
#	src-docs/src/i18ntokens.json
@cchaos
Copy link
Contributor Author

cchaos commented Jul 30, 2019

Pushed the combobox fix

@snide
Copy link
Contributor

snide commented Jul 30, 2019

For another PR, but I think given how we've used these within panels, we should either made a compressed mode for EuiExpression or just make the default smaller.

image

@snide
Copy link
Contributor

snide commented Jul 30, 2019

Form control layout changes doing some funky stuff on inline datepickers.

image

image

@snide
Copy link
Contributor

snide commented Jul 30, 2019

Color picker compressed shadows

image

@cchaos
Copy link
Contributor Author

cchaos commented Jul 30, 2019

Yes, the expression styles will come in another PR.

I have pushed fixes for the date pickers and color picker

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.

Some small code comments. Looks pretty clean.

@@ -80,8 +82,12 @@ export class FilePicker extends Component {
<EuiSpacer size="m" />

<EuiFilePicker
id="asdf4"
Copy link
Contributor

Choose a reason for hiding this comment

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

🤷‍♂

}
}
}

@mixin euiFormControlWithIcon($isIconOptional: false, $side: 'left') {
@mixin euiFormControlWithIcon($isIconOptional: false, $side: 'left', $compressed: false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@@ -172,25 +167,49 @@
border-radius: 0;
padding: $euiFormControlPadding;

@if ($includeStates) {
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

src/components/form/file_picker/_file_picker.scss Outdated Show resolved Hide resolved

&:disabled {
background-color: $euiFormBackgroundDisabledColor;
color: $euiFormControlDisabledColor; // ensures established contrast
}

// sass-lint:disable-block no-important
// This is the only way to target specific components to override styling
Copy link
Contributor

Choose a reason for hiding this comment

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

🚶 👁

Copy link
Contributor

Choose a reason for hiding this comment

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

^^ Just a reminder to @thompsongl that occasionally this type of thing exists in EUI. It is very, very rare. Just another hard problem to figure out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah so the only other way I can of to not have this, is to be more strict in the append/prepend props by only allowing them to pass something like:

prepend: {
  button: "Some text for button",
  iconButton: "iconType",
  label: "Some text for a label",
  popover: meh?
}

But right now the component allows you to pass any number of React.elements which is the most flexible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We've decided that we will leave this as-is for now and revisit before the feature branch merges.

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 all looks great, can't wait to use it!

From a visual review standpoint, the alignment of input text alongside and icons/labels in prepend/append buttons stands out though the code looks good (even padding; line-height match input heights, etc.).

I'm not certain there is anything more to be done here that isn't rather hacky with regard to height and line-height (i.e. likely just how Inter UI is designed to some degree), but here is the current state where the prepend icon looks a bit low relative to button label and button label and input text look top-aligned (I realize they are not actually top aligned, just stating what it looks like visually):

Screenshot 2019-07-30 10 37 33

It would be tidier looking if it looked more like this, but this is the result of some unacceptable nudging with odd height and such (just to demonstrate):

Screenshot 2019-07-30 10 36 57

Take or leave this feedback, not sure how actionable it is in reality.

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.

LGTM. This stuff looks great.

@cchaos
Copy link
Contributor Author

cchaos commented Jul 30, 2019

Thanks @ryankeairns It was bugging me too, but couldn't find a good solution. However, since you brought it up I decided to look into it again and I think I found a solution, though maybe a weird one. Essentially I had a 16px line-height added to all the prepend/appends but if I changed that to 15px all the elements seem to line up better:

Screen Shot 2019-07-30 at 11 56 04 AM

I'll push the fix. I checked in FF and IE again as well, and all looks swell.

@ryankeairns ryankeairns self-requested a review July 30, 2019 16:25
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.

@cchaos nice! That is essentially how I hacked together the screenshot 😬 Thanks for making the change.

@snide
Copy link
Contributor

snide commented Jul 30, 2019

but if I changed that to 15px all the elements seem to line up better

I know I had transforms(-1px) on some icons back in the day. I wouldn't be surprised if it's related. I kind of don't remember where though.

@cchaos
Copy link
Contributor Author

cchaos commented Jul 30, 2019

Force retest to try to trigger the report back, Jenkins, test this

@cchaos cchaos merged commit 3637a74 into elastic:feature/compressed-editor-controls Jul 30, 2019
@cchaos cchaos deleted the into-feature/compressed-styles branch July 30, 2019 17:38
cchaos added a commit that referenced this pull request Aug 13, 2019
* 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
cchaos added a commit that referenced this pull request Aug 29, 2019
* 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
cchaos added a commit that referenced this pull request Aug 30, 2019
* 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
@snide snide mentioned this pull request Sep 5, 2019
13 tasks
cchaos added a commit that referenced this pull request Sep 9, 2019
* 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
thompsongl pushed a commit to thompsongl/eui that referenced this pull request Sep 10, 2019
* 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
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
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.

5 participants