Upgrading EUI to the new Amsterdam theme

[cchaos]

EUI is taking the opportunity in the new theme to increase the flexibility in styling without breaking the previous theme. This means that while the component is working correctly in the default theme, you may need to adjust some props for the new theme.

We'll use this discussion to list the known breaks/upgrade paths for consumers of Amsterdam.

[cchaos]

EuiPopover

EuiPopover’s paddingSize prop in Amsterdam affects all popover components (title, panel, footer). Most usages of EuiPopover that removed this padding will need to add props to the inner components (the popover header and footer) directly.

[cchaos]

EuiText

Amsterdam now comes with a smaller body font size, down from 16px to 14px. Most EUI components specifically set font sizing to accommodate for this change. However, if you were relying on the larger 16px font size by default without using EuiText as a wrapper, this will now be the smaller 14px.

The solution, for those who need the larger font size, should wrap their text with an EuiText block with size="m".

[cchaos]

EuiPanel

In the default theme, EuiPanel comes with a border and a slight shadow by default. In Amsterdam, this border has been removed. However, if the intended style of the panel was to have the border, you can add it back on with hasBorder={true}. This will, however, remove the shadow, as Amsterdam no longer supports both hasShadow and hasBorder together.

Default theme panel

Amsterdam default panel

[cchaos]

EuiFilterButton

This affects both themes.

Filter buttons need to be encompassed inside a EuiFilterGroup even if there is only one button. Otherwise you will see missing borders like this:

To fix, just wrap your buttons (and popovers) with the group like so:

<EuiFilterGroup>
  <EuiPopover button={<EuiFilterButton />} />
</EuiFilterGroup>

[cchaos]

Focus outline on non-buttons

Amsterdam focuses on increasing legibility of focus states for all components. It adds a general outline onto all interact-able elements when the keyboard is being used to navigate.

eui/src/themes/eui-amsterdam/global_styling/reset/_reset.scss

Lines 72 to 89 in 7dc7da7

	*:focus {
	// 👉 Safari and Firefox innately respect only showing the outline with keyboard only
	// 💔 But they don't allow coloring of the 'auto'/default outline, so contrast is no good in dark mode.
	// 👉 For these browsers we use the solid type in order to match with `currentColor`.
	// 😦 Which does means the outline will be square
	outline: $euiFocusRingSize solid currentColor;
	outline-offset: -($euiFocusRingSize / 2);
	// 👀 Chrome respects :focus-visible and allows coloring the `auto` style
	&:focus-visible {
	outline-style: auto;
	}
	//🙅‍♀️ But Chrome also needs to have the outline forcefully removed from regular `:focus` state
	&:not(:focus-visible) {
	outline: none;
	}
	}

This may mean that some non-button elements like EuiFlyout and EuiPopover that get a tabindex property for keyboard focus, will get focus rings as well.

Here is a list of known components and any progress for updating these:

• EuiPopover: Done [Amsterdam] Fixing some focus states #4876
• EuiFlyout: Done Expand EuiFlyout usage to help EuiCollapsibleNav #4713
• EuiFlyoutBody: Done Expand EuiFlyout usage to help EuiCollapsibleNav #4713

Please add to the list above as more are found.

[myasonik]

For Flyouts (and others things) we should have a visible focus indicator if it's scrollable. (See previous similar discussion for code blocks.)

Right now, Webkit and Chromium browsers don't do this automatically but Firefox does. So fixing this in Webkit and Chromium is up to the developer, but not breaking this in Firefox seems equally, if not even more, valuable.

It'll be a little more JS, I think, but how do you feel about adding a "is the flyout currently scrollable?" check before removing the outline?

[cchaos]

Yep, we will need to add a focus ring to the inner EuiFlyoutBody, where the actual scroll exists, but we need to remove it from the entire EuiFlyout which does not scroll on its own. Removing outline from EuiFlyout should get rid of the unnecessary focus ring that exists from tabindex.