[BUG] Fix the `EuiRange` prepend prop CSS #7678

1Copenut · 2024-04-11T21:53:32Z

Summary

PR closes #7677.

I updated the CSS using Emotion and euiThemeProvider to fix the incorrect background from showing through.

Before:

After:

Reviewer QA

Download the branch
yarn && yarn start
Verify the fix. I had to use the && to double up classname and get specificity for the readOnly. Tested on all 4 evergreen browser.

QA

Remove or strikethrough items that do not apply to your PR.

General checklist

Browser QA
- Checked in both light and dark modes
- Checked in mobile
- Checked in Chrome, Safari, Edge, and Firefox
Docs site QA
- Checked Code Sandbox works for any docs examples
Code quality checklist
- Added or updated jest and cypress tests

* Added CSS for disabled, readOnly states when input is prepended. * Updated one snapshot for disabled prop.

1Copenut

Two clarifying comments, one screenshot.

src/components/form/range/range_wrapper.styles.ts

1Copenut · 2024-04-11T21:59:38Z

src/components/form/range/range_wrapper.tsx

+      disabled,
+      readOnly,


Only added this change to the single EuiRange but tested it in all examples on the docs site. Wasn't able to get an EuiDualRange to show the prepend or append labels, but would be an easy add if I just missed something.

Lol I'm honestly still so confused why our single ranges even support append/prepend 🙈 Ah well, probably too much of a hassle to breaking change/remove it now

mgadewoll

🤔 I'm wondering if the continuous growing behavior is expected? The wider the prepend content, the more overflow is visible on the right as the input on the left has a fixed width and the group flex container extends.

Maybe the prepend should have a more strict fixed width than the current max-width: 50%? Should they always be the same width? (I'm missing historic insights here though)
cc @cee-chen

src/components/form/range/range_wrapper.styles.ts

cee-chen · 2024-04-12T17:23:57Z

🤔 I'm wondering if the continuous growing behavior is expected? The wider the prepend content, the more overflow is visible on the right as the input on the left has a fixed width and the group flex container extends.

I'm kind of surprised we allow a prepend/append on EuiRange at all 😅 What's the use case for it that you have in mind @1Copenut?

Maybe the prepend should have a more strict fixed width than the current max-width: 50%? Should they always be the same width? (I'm missing historic insights here though)

In general I do think the range input's width needs to be considered with how it impacts the range slider, but we don't need to try and limit this ahead of time. Consumers know better than we do how their content relates to their available space/layouts, and as long as we give them the tools needed (e.g. fullWidth props, custom CSS) to handle layout edge cases I don't feel it necessary to try and bake in guard rails for extreme edge cases.

cee-chen · 2024-04-12T17:51:35Z

OK disregard literally every comment I left before this, I'm so sorry. I just had time to sit down and actually inspect the DOM and what's happening and this isn't a background color issue, it's a flex / width issue. Here's what's happening:

eui/src/components/form/range/range_input.tsx

Lines 85 to 87 in 976e603

    
           return logicalStyles({ 
        
             width: `calc(${inputPadding} + ${inputCharWidth}ch + ${stepperWidth}em + ${invalidIconWidth}px)`, 
        
           });

We're setting a max-width (inline-size) on the input via inline style which is what's causing the gapping

the prepend/append labels have a max-width: 50% CSS on them as Lene noted, and if you remove this you'll note this "fixes" the issue (because the flex parent is going off of the total width of the untruncated text)

The solution here for range inputs only is to override the % based max-width on prepend/appends, because that's what's messing things up. I'll look into something that makes more sense here in a bit and push something up if that's cool @1Copenut

1Copenut · 2024-04-12T18:01:07Z

I'll look into something that makes more sense here in a bit and push something up if that's cool @1Copenut

@cee-chen Yes, I'm 100% cool with that!

…ppend labels

cee-chen

@mgadewoll @1Copenut feel free to re-review/QA before merging, and let me know if any of the comments don't make sense!

cee-chen · 2024-04-12T18:54:53Z

src/components/form/range/range_wrapper.styles.ts

+      /* TODO: When converting forms to Emotion, allow passing wrapperProps
+         to EuiFieldNumber and then move this CSS to range_input.styles.ts */


This TODO is for myself once I start converting forms to Emotion

cee-chen · 2024-04-12T18:55:25Z

changelogs/upcoming/7678.md

+
+- Visual fixes for `EuiRange`s with `showInput`:
+  - Longer `append`/`prepend` labels no longer cause a background bug
+  - Inputs can no longer overwhelm the actual range in width


Thanks to @mgadewoll for pointing out this visual bug and getting me on the right track of the underlying bug!

kibanamachine · 2024-04-12T19:20:50Z

Preview staging links for this PR:

Docs site: https://eui.elastic.co/pr_7678/
Storybook: https://eui.elastic.co/pr_7678/storybook

elasticmachine · 2024-04-12T19:20:53Z

💚 Build Succeeded

Buildkite Build
Commit: b620a63

History

💚 Build #1741 succeeded 70fb806
💔 Build #1734 failed d36573f

cc @1Copenut

1Copenut · 2024-04-12T19:54:19Z

I'm comfortable merging this as is. I feel we've carefully reviewed the problem, improved on the original solution, and scoped the change to this one component.

`v94.1.0-backport.0` ⏩ `v94.2.1-backport.0` _[Questions? Please see our Kibana upgrade FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)_ --- ## [`v94.2.1-backport.0`](https://github.com/elastic/eui/releases/v94.2.1-backport.0) **This is a backport release only intended for use by Kibana.** - Reverted the `EuiFlexGroup`/`EuiFlexItem` `component` prop feature due to Kibana typing issues ## [`v94.2.1`](https://github.com/elastic/eui/releases/v94.2.1) **Bug fixes** - Fixed an `EuiTabbedContent` edge case bug that occurred when updated with a completely different set of `tabs` ([#7713](elastic/eui#7713)) - Fixed the `@storybook/test` dependency to be listed in `devDependencies` and not `dependencies` ([#7719](elastic/eui#7719)) ## [`v94.2.0`](https://github.com/elastic/eui/releases/v94.2.0) - Updated `getDefaultEuiMarkdownPlugins()` to allow excluding the following plugins in addition to `tooltip`: ([#7676](elastic/eui#7676)) - `checkbox` - `linkValidator` - `lineBreaks` - `emoji` - Updated `EuiSelectable`'s `isPreFiltered` prop to allow passing a configuration object, which allows disabling search highlighting in addition to search filtering ([#7683](elastic/eui#7683)) - Updated `EuiFlexGroup` and `EuiFlexItem` prop types to support passing any valid React component type to the `component` prop and ensure proper type checking of the extra props forwarded to the `component`. ([#7688](elastic/eui#7688)) - Updated `EuiSearchBar` to allow the `@` special character in query string searches ([#7702](elastic/eui#7702)) - Added a new, optional `optionMatcher` prop to `EuiSelectable` and `EuiComboBox` allowing passing a custom option matcher function to these components and controlling option filtering for given search string ([#7709](elastic/eui#7709)) **Bug fixes** - Fixed an `EuiPageTemplate` bug where prop updates would not cascade down to child sections ([#7648](elastic/eui#7648)) - To cascade props down to the sidebar, `EuiPageTemplate` now explicitly requires using the `EuiPageTemplate.Sidebar` rather than `EuiPageSidebar` - Fixed `EuiFieldNumber`'s typing to accept an icon configuration shape ([#7666](elastic/eui#7666)) - Fixed `EuiFieldText` and `EuiFieldNumber` to render the correct paddings for icon shapes set to `side: 'right'` ([#7666](elastic/eui#7666)) - Fixed `EuiFieldText` and `EuiFieldNumber` to fully ignore `icon`/`prepend`/`append` when `controlOnly` is set to true ([#7666](elastic/eui#7666)) - Fixed `EuiColorPicker`'s input not setting the correct right padding for the number of icons displayed ([#7666](elastic/eui#7666)) - Visual fixes for `EuiRange`s with `showInput`: ([#7678](elastic/eui#7678)) - Longer `append`/`prepend` labels no longer cause a background bug - Inputs can no longer overwhelm the actual range in width - Fixed a visual text alignment regression in `EuiTableRowCell`s with the `row` header scope ([#7681](elastic/eui#7681)) - Fixed `toolTipProps` type on `EuiSuperUpdateButton` to use `Partial<EuiToolTipProps>` ([#7692](elastic/eui#7692)) - Fixes missing prop type for `popperProps` on `EuiDatePicker` ([#7694](elastic/eui#7694)) - Fixed a focus bug with `EuiDataGrid`s with `leadingControlColumns` when moving columns to the left/right ([#7701](elastic/eui#7701)) ([#7698](elastic/eui#7698)) - Fixed `EuiSuperDatePicker` to validate date string with respect of locale on `EuiAbsoluteTab`. ([#7705](elastic/eui#7705)) - Fixed a visual bug with `EuiSuperDatePicker`'s absolute tab on small mobile screens ([#7708](elastic/eui#7708)) - Fixed i18n of empty and loading state messages for the `FieldValueSelectionFilter` component ([#7718](elastic/eui#7718)) **Dependency updates** - Updated `@hello-pangea/dnd` to v16.6.0 ([#7599](elastic/eui#7599)) - Updated `remark-rehype` to v8.1.0 ([#7601](elastic/eui#7601)) **Accessibility** - Improved `EuiBasicTable` and `EuiInMemoryTable`'s selection checkboxes to have unique aria-labels per row ([#7672](elastic/eui#7672)) - Added `aria-valuetext` attributes to `EuiRange`s with tick labels for improved screen reader UX ([#7675](elastic/eui#7675)) - Updated `EuiAccordion` to keep focus on accordion trigger instead of moving to content on click/keypress ([#7696](elastic/eui#7696)) - Added `aria-disabled` attribute to `EuiHorizontalSteps` when status is "disabled" ([#7699](elastic/eui#7699)) --------- Co-authored-by: Tomasz Kajtoch <[email protected]>

1Copenut added 2 commits April 11, 2024 16:18

[BUG][EuiRange]: Fixed input background color

f26a11c

* Added CSS for disabled, readOnly states when input is prepended. * Updated one snapshot for disabled prop.

Removed important and used Emotmotion specificity override instead.

d36573f

1Copenut added bug skip-changelog labels Apr 11, 2024

1Copenut self-assigned this Apr 11, 2024

1Copenut requested a review from a team as a code owner April 11, 2024 21:53

1Copenut commented Apr 11, 2024

View reviewed changes

mgadewoll reviewed Apr 12, 2024

View reviewed changes

src/components/form/range/range_wrapper.styles.ts Outdated Show resolved Hide resolved

Switching to more specific background-color property.

70fb806

Revert changes

2c388d0

cee-chen added 2 commits April 12, 2024 11:51

Fix flex/width relationship of EuiRange inputs, ranges, and prepend/a…

75e35e3

…ppend labels

changelog

b620a63

cee-chen approved these changes Apr 12, 2024

View reviewed changes

1Copenut merged commit 66a5cf8 into elastic:main Apr 15, 2024
7 checks passed

1Copenut deleted the bug/euiRangeInput-prepend-css branch April 15, 2024 14:09

cee-chen mentioned this pull request Apr 29, 2024

Upgrade EUI to v94.2.1 elastic/kibana#182023

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG] Fix the `EuiRange` prepend prop CSS #7678

[BUG] Fix the `EuiRange` prepend prop CSS #7678

1Copenut commented Apr 11, 2024 •

edited

Loading

1Copenut left a comment

1Copenut Apr 11, 2024

cee-chen Apr 12, 2024

mgadewoll left a comment

cee-chen commented Apr 12, 2024 •

edited

Loading

cee-chen commented Apr 12, 2024

1Copenut commented Apr 12, 2024

cee-chen left a comment

cee-chen Apr 12, 2024

cee-chen Apr 12, 2024

kibanamachine commented Apr 12, 2024

elasticmachine commented Apr 12, 2024

1Copenut commented Apr 12, 2024

		/* TODO: When converting forms to Emotion, allow passing wrapperProps
		to EuiFieldNumber and then move this CSS to range_input.styles.ts */

[BUG] Fix the EuiRange prepend prop CSS #7678

[BUG] Fix the EuiRange prepend prop CSS #7678

Conversation

1Copenut commented Apr 11, 2024 • edited Loading

Summary

Reviewer QA

QA

General checklist

1Copenut left a comment

Choose a reason for hiding this comment

1Copenut Apr 11, 2024

Choose a reason for hiding this comment

cee-chen Apr 12, 2024

Choose a reason for hiding this comment

mgadewoll left a comment

Choose a reason for hiding this comment

cee-chen commented Apr 12, 2024 • edited Loading

cee-chen commented Apr 12, 2024

1Copenut commented Apr 12, 2024

cee-chen left a comment

Choose a reason for hiding this comment

cee-chen Apr 12, 2024

Choose a reason for hiding this comment

cee-chen Apr 12, 2024

Choose a reason for hiding this comment

kibanamachine commented Apr 12, 2024

elasticmachine commented Apr 12, 2024

💚 Build Succeeded

History

1Copenut commented Apr 12, 2024

[BUG] Fix the `EuiRange` prepend prop CSS #7678

[BUG] Fix the `EuiRange` prepend prop CSS #7678

1Copenut commented Apr 11, 2024 •

edited

Loading

cee-chen commented Apr 12, 2024 •

edited

Loading