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.

Sign up for GitHub

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

Jump to bottom

[EuiSuperDatePicker] Fix incorrect button margins in quick select popover #6380

Merged
merged 6 commits into from
Nov 17, 2022
Merged

[EuiSuperDatePicker] Fix incorrect button margins in quick select popover #6380

merged 6 commits into from
Nov 17, 2022

Conversation

miukimiu
Copy link
Contributor

@miukimiu miukimiu commented Nov 17, 2022 •
edited
Loading

Summary

This PR fixes incorrect button margins in EuiSuperDatePicker quick select popover. This bug was introduced when the EuiFlexGrid was converted to Emotion.

super-date-picker@2x

How to test

Compare the example with the bug with the fix:

QA

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

General checklist

  • Checked in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • A changelog entry exists and is marked appropriately

@miukimiu miukimiu added the bug label Nov 17, 2022
miukimiu and others added 3 commits November 17, 2022 17:16
@miukimiu
Adding CL
45e4b08
@cee-chen
Remove now-unnecessary negative margin workarounds
b6b7b93
@cee-chen
Tweak changelog
f2722ae 
- more generic since we're fixing multiple margins, label cause of bugs in case it's handy to search for in the future
cee-chen
cee-chen reviewed Nov 17, 2022
View reviewed changes
src/components/date_picker/super_date_picker/_super_date_picker.scss
Comment on lines -4 to -5
// Need to offset 8px because of negative margins added by small size gutter
max-width: calc(100% + #{$euiSizeS});
Copy link
Member

@cee-chen cee-chen Nov 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Just for context - I spotted this issue in Kibana vs. EUI's docs - not 100% sure what the difference is in our implementation, but here's the before and after:

Before

After

Copy link
Contributor Author

@miukimiu miukimiu Nov 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I think because they're not actually using our update button. They are rendering a custom one:
https://github.com/elastic/kibana/blob/75128bbc4b44571bb807344714ec217b9957e78c/src/plugins/unified_search/public/query_string_input/query_bar_top_row.tsx#L461

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha, thanks!! This fix doesn't seem to affect our EUI docs either way so I think is fairly safe to do

@kibanamachine
Copy link

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

cee-chen
cee-chen reviewed Nov 17, 2022
View reviewed changes
src/components/date_picker/super_date_picker/quick_select_popover/_quick_select_popover.scss Outdated
Comment on lines 28 to 30
&:not(:last-of-type) {
margin-bottom: $euiSizeS;
}
Copy link
Member

@cee-chen cee-chen Nov 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@miukimiu this fix for the commonly used section unfortunately creates a visual bug in the recently used section:

I'm pushing up a fix now to add a modifier class for recently used items

edit: 293ea9f

@cee-chen
Remove last unnecessary negative margin accommodation + smooth out se…
124da53 
…ction spacing

- using margin instead of padding makes the scroll experience nicer for recently used ranges

- this also makes it so custom panels don't need the spacer, it already comes from margin
@cee-chen
Copy link
Member

@miukimiu I've QA'd fairly thoroughly on mobile mode at this point and I think this is good to go! Are you good with this?

@kibanamachine
Copy link

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

@cee-chen cee-chen marked this pull request as ready for review November 17, 2022 20:32
@cee-chen cee-chen merged commit 78546e5 into elastic:main Nov 17, 2022
@cee-chen cee-chen mentioned this pull request Nov 17, 2022
Merged
jbudz pushed a commit to elastic/kibana that referenced this pull request Nov 18, 2022
@miukimiu @kibanamachine
Upgrade EUI to v70.2.4 (#144845)
c408ceb 
## Summary
`eui@67.1.8` ⏩ `eui@70.2.2`

⚠️ Note: This upgrade contains breaking changes to `EuiFlexGroup` and
`EuiFlexGrid`, primarily around switching margins and negative margins
to `gap`. Please do a quick QA pass of your app to scan for any issues.
We're happy to help resolve minor fixes, or potentially follow up after
PR merges. You can find us over in #eui!

## [`70.2.4`](https://github.com/elastic/eui/tree/v70.2.4)

**Bug fixes**

- Fixed visual bug in nested `EuiFlexGroup`s, where the parent
`EuiFlexGroup` is responsive but a child `EuiFlexGroup` is not
([#6381](elastic/eui#6381))

## [`70.2.3`](https://github.com/elastic/eui/tree/v70.2.3)

**Bug fixes**

- Fixed incorrect margins in `EuiSuperDatePicker` caused by `EuiFlex`
CSS gap change ([#6380](elastic/eui#6380))

## [`70.2.2`](https://github.com/elastic/eui/tree/v70.2.2)

- `EuiButton` now accepts `minWidth={false}`
([#6373](elastic/eui#6373))

**Bug fixes**

- `EuiButton` no longer outputs unnecessary inline styles for
`minWidth={0}` or `minWidth={false}`
([#6373](elastic/eui#6373))
- `EuiFacetButton` no longer reports type issues when passing props
accepted by `EuiButton`
([#6373](elastic/eui#6373))
- Fixed the shadow sizes of `.eui-yScrollWithShadows` and
`.eui-xScrollWithShadows`
([#6374](elastic/eui#6374))


## [`70.2.1`](https://github.com/elastic/eui/tree/v70.2.1)

**Bug fixes**

- Re-fixed `EuiPageSection` not correctly merging `contentProps.css`
([#6365](elastic/eui#6365))
- Fixed `EuiTab` not defaulting to size `m`
([#6366](elastic/eui#6366))

## [`70.2.0`](https://github.com/elastic/eui/tree/v70.2.0)

- Added a keyboard shortcuts popover to `EuiDataGrid`'s toolbar. This
can be visually hidden via `toolbarVisibility.showKeyboardShortcuts`,
but will always remain accessible to keyboard and screen reader users.
([#6036](elastic/eui#6036))
- `EuiScreenReaderOnly`'s `showOnFocus` prop now also shows on focus
within its children ([#6036](elastic/eui#6036))
- Added `onFocus` prop callback to `EuiSuperDatePicker`
([#6320](elastic/eui#6320))

**Bug fixes**

- Fixed `EuiSelectable` to ensure the full options list is re-displayed
when the search bar is controlled and cleared using `searchProps.value`
([#6317](elastic/eui#6317))
- Fixed incorrect padding on `xl`-sized `EuiTabs`
([#6336](elastic/eui#6336))
- Fixed `EuiCard` not correctly merging `css` on its child `icon`s
([#6341](elastic/eui#6341))
- Fixed `EuiCheckableCard` not setting `css` on the correct DOM node
([#6341](elastic/eui#6341))
- Fixed a webkit rendering issue with `EuiModal`s containing
`EuiBasicTable`s tall enough to scroll
([#6343](elastic/eui#6343))
- Fixed bug in `to_initials` that truncates custom initials
([#6346](elastic/eui#6346))
- Fix bug in `EuiCard` where layout breaks when `horizontal` and
`selectable` are both passed
([#6348](elastic/eui#6348))

## [`70.1.0`](https://github.com/elastic/eui/tree/v70.1.0)

- Added the `hint` prop to the `<EuiSearchBar />`. This prop lets the
consumer render a hint below the search bar that will be displayed on
focus. ([#6319](elastic/eui#6319))
- Added the `hasDragDrop` prop to `EuiPopover`. Use this prop if your
popover contains `EuiDragDropContext`.
([#6329](elastic/eui#6329))

**Bug fixes**

- Fixed `EuiButton`'s cursor style when the button is disabled
([#6323](elastic/eui#6323))
- Fixed `EuiPageTemplate` not recognizing child
`EuiPageSidebar`s/`EuiPageTemplate.Sidebar`s with `css` props
([#6324](elastic/eui#6324))
- Fixed `EuiBetaBadge` to always respect its `anchorProps` values,
including when there is no tooltip content
([#6326](elastic/eui#6326))
- Temporarily patched `EuiModal` to not cause scroll-jumping issues on
modal open ([#6327](elastic/eui#6327))
- Fixed buggy drag & drop behavior within `EuiDataGrid`'s columns &
sorting toolbar popovers
([#6329](elastic/eui#6329))
- Fixed `EuiButton` not correctly passing `textProps` for children
inside fragments or i18n components
([#6332](elastic/eui#6332))
- Fixed `EuiButton` not correctly respecting `minWidth={0}`
([#6332](elastic/eui#6332))

**CSS-in-JS conversions**

- Converted `EuiTabs` to Emotion
([#6311](elastic/eui#6311))

## [`70.0.0`](https://github.com/elastic/eui/tree/v70.0.0)

- Added the `enabled` option to the `<EuiInMemoryTable />`
`executeQueryOptions` prop. This option prevents the Query from being
executed when controlled by the consumer.
([#6284](elastic/eui#6284))

**Bug fixes**

- Fixed `EuiOverlayMask` to set a
`[data-relative-to-header=above|below]` attribute to replace the
`--aboveHeader` and `--belowHeader` classNames removed in its Emotion
conversion ([#6289](elastic/eui#6289))
- Fixed `EuiHeader` CSS using removed `EuiOverlayMask` class modifiers
([#6293](elastic/eui#6293))
- Fixed `EuiToolTip` not respecting reduced motion preferences
([#6295](elastic/eui#6295))
- Fixed a bug with `EuiTour` where passing any `panelProps` would cause
the beacon to disappear
([#6298](elastic/eui#6298))

**Breaking changes**

- `@emotion/css` is now a required peer dependency, alongside
`@emotion/react` ([#6288](elastic/eui#6288))
- `@emotion/cache` is no longer required peer dependency, although your
project must still use it if setting custom cache/injection locations
([#6288](elastic/eui#6288))

**CSS-in-JS conversions**

- Converted `EuiCode` and `EuiCodeBlock` to Emotion; Removed
`euiCodeSyntaxTokens` Sass mixin and `$euiCodeBlockPaddingModifiers`;
([#6263](elastic/eui#6263))
- Converted `EuiResizableContainer` and `EuiResizablePanel` to Emotion
([#6287](elastic/eui#6287))

## [`69.0.0`](https://github.com/elastic/eui/tree/v69.0.0)

- Added support for `fullWidth` prop on EuiForm, which will be the
default for all rows/controls within
([#6229](elastic/eui#6229))
- Added support for `onResizeStart` and `onResizeEnd` callbacks to
`EuiResizableContainer`
([#6236](elastic/eui#6236))
- Added optional case sensitive option matching to `EuiComboBox` with
the `isCaseSensitive` prop
([#6268](elastic/eui#6268))
- `EuiFlexItem` now supports `grow={0}`
([#6270](elastic/eui#6270))
- Added the `alignItems` prop to `EuiFlexGrid`
([#6281](elastic/eui#6281))
- Added `filter`, `filterExclude`, `filterIgnore`, `filterInclude`,
`indexTemporary`, `infinity`, `sortAscending`, and `sortDescending`
glyphs to `EuiIcon` ([#6282](elastic/eui#6282))

**Bug fixes**

- Fixed `EuiTextProps` to show the `color` type option `inherit` as
default ([#6267](elastic/eui#6267))
- `EuiFlexGroup` now correctly respects `gutterSize` when responsive
([#6270](elastic/eui#6270))
- Fixed the last breadcrumb in `EuiBreadcrumbs`'s `breadcrumbs` array
not respecting `truncate` overrides
([#6280](elastic/eui#6280))

**Breaking changes**

- `EuiFlexGrid` no longer supports `columns={0}`. Use `EuiFlexGroup`
instead for normal flex display
([#6270](elastic/eui#6270))
- `EuiFlexGrid` now uses modern `display: grid` CSS
([#6270](elastic/eui#6270))
- `EuiFlexGroup`, `EuiFlexGrid`, and `EuiFlexItem` now use modern `gap`
CSS instead of margins and negative margins
([#6270](elastic/eui#6270))
- `EuiFlexGroup` no longer applies responsive styles to `column` or
`columnReverse` directions
([#6270](elastic/eui#6270))

**CSS-in-JS conversions**

- Converted `EuiFlexGroup`, `EuiFlexGrid`, and `EuiFlexItem` to Emotion
([#6270](elastic/eui#6270))

## [`68.0.0`](https://github.com/elastic/eui/tree/v68.0.0)

- Added `beta` glyph to `EuiIcon`
([#6250](elastic/eui#6250))
- Added `launch` and `spaces` glyphs to `EuiIcon`
([#6260](elastic/eui#6260))
- Added the `fallbackDestination` prop to `EuiSkipLink`, which accepts a
string of query selectors to fall back to if the `destinationId` does
not have a valid target. Defaults to `main`
([#6261](elastic/eui#6261))
- `EuiSkipLink` is now always an `a` tag to ensure that it is always
placed within screen reader link menus.
([#6261](elastic/eui#6261))

**Bug fixes**

- Fixed `EuiSuperDatePicker` not correctly merging passed `className`s
([#6253](elastic/eui#6253))
- Fixed `EuiColorStops` not correctly merging in passed
`data-test-subj`s, `style`s, or `...rest`
([#6255](elastic/eui#6255))
- Fixed `EuiResizablePanel` incorrectly passing `style` to the wrapper
instead of the panel. Use `wrapperProps.style` to pass styles to the
wrapper. ([#6255](elastic/eui#6255))
- Fixed custom `onClick`s passed to `EuiSkipLink` overriding
`overrideLinkBehavior`
([#6261](elastic/eui#6261))

**Breaking changes**

- Removed `inherit` and `ghost` color from `EuiListGroupItem`
([#6207](elastic/eui#6207))
- Changed default color to `text` instead of `inherit`
([#6207](elastic/eui#6207))

**CSS-in-JS conversions**

- Converted `EuiListGroup` and `EuiListGroupItem` to Emotion; Removed
`$euiListGroupGutterTypes`, `$euiListGroupItemColorTypes` and
`$euiListGroupItemSizeTypes`;
([#6207](elastic/eui#6207))
- Converted `EuiBadgeGroup` to Emotion
([#6258](elastic/eui#6258))
- Converted `EuiBetaBadge` to Emotion
([#6258](elastic/eui#6258))
- Converted `EuiNotificationBadge` to Emotion
([#6258](elastic/eui#6258))

Co-authored-by: Elizabet Oliveira <elizabet.oliveira@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@1Copenut 1Copenut mentioned this pull request Nov 21, 2022
Merged
@cee-chen cee-chen mentioned this pull request Nov 28, 2022
Merged
6 tasks
1Copenut added a commit to elastic/kibana that referenced this pull request Dec 2, 2022
@1Copenut @cee-chen
Bumping EUI to v70.4.0 (#145888)
7376d9c 
`eui@70.2.4` ⏩ `eui@70.4.0`

- "Fixed EuiButtonGroup firing onChange twice" required changing some
tests from `click` to `change`
___

## [`70.4.0`](https://github.com/elastic/eui/tree/v70.4.0)

- Updated `EuiTourStep.footerAction` type to accept `ReactNode[]`
([#6384](elastic/eui#6384))
- Vertically aligned all footer content so that `euiTourStepIndicator`
is always centered ([#6384](elastic/eui#6384))
- Added `filterInCircle` glyph to `EuiIcon`
([#6385](elastic/eui#6385))
- Added `color` prop to `EuiBeacon`
([#6420](elastic/eui#6420))
- Added the `euiMaxBreakpoint` and `euiMinBreakpoint` CSS-in-JS
utilities for creating min/max-width media queries
([#6431](elastic/eui#6431))

**Bug fixes**

- Restores the previous match operator behaviour when the query value is
split into multiple terms after analysis.
([#6409](elastic/eui#6409))
- Fixed missing slide-in animation on `EuiCollapsibleNav`s and left-side
`EuiFlyout`s ([#6422](elastic/eui#6422))
- Fix bug in `EuiCard` where footer were not aligned to the bottom of
the card ([#6424](elastic/eui#6424))
- Fixed multiple component media queries for consumers with custom theme
breakpoints ([#6431](elastic/eui#6431))

## [`70.3.0`](https://github.com/elastic/eui/tree/v70.3.0)

- `EuiSearchBar` now automatically wraps special characters not used by
query syntax in quotes
([#6356](elastic/eui#6356))
- Added `alignment` prop to `EuiBetaBadge`
([#6361](elastic/eui#6361))
- `EuiButton` now accepts `minWidth={false}`
([#6373](elastic/eui#6373))

**Bug fixes**

- Fixed `EuiPageTemplate` not correctly passing the `component` prop to
the inner main content wrapper.
([#6352](elastic/eui#6352))
- `EuiSkipLink` now correctly calls `onClick` even when
`fallbackDestination` is invalid
([#6355](elastic/eui#6355))
- Permanently fixed `EuiModal` to not cause scroll-jumping issues on
modal open ([#6360](elastic/eui#6360))
- Re-fixed `EuiPageSection` not correctly merging `contentProps.css`
([#6365](elastic/eui#6365))
- Fixed `EuiTab` not defaulting to size `m`
([#6366](elastic/eui#6366))
- Fixed the shadow sizes of `.eui-yScrollWithShadows` and
`.eui-xScrollWithShadows`
([#6374](elastic/eui#6374))
- Fixed bug in `EuiCard` where the inner content in vertical cards was
not growing 100% in width
([#6377](elastic/eui#6377))
- Fixed incorrect margins in `EuiSuperDatePicker` caused by `EuiFlex`
CSS gap change ([#6380](elastic/eui#6380))
- Fixed visual bug in nested `EuiFlexGroup`s, where the parent
`EuiFlexGroup` is responsive but a child `EuiFlexGroup` is not
([#6381](elastic/eui#6381))

**CSS-in-JS conversions**

- Converted `EuiModal` to Emotion
([#6321](elastic/eui#6321))

**Fixes**

- `EuiButton` no longer outputs unnecessary inline styles for
`minWidth={0}` or `minWidth={false}`
([#6373](elastic/eui#6373))
- `EuiFacetButton` no longer reports type issues when passing props
accepted by `EuiButton`
([#6373](elastic/eui#6373))

Co-authored-by: Constance Chen <constance.chen@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cee-chen cee-chen cee-chen approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@miukimiu @kibanamachine @cee-chen