Initial Amsterdam Setup

[johnbarrierwilson]

Summary

As part of the "Break Stuff for 8.0" design theme, this PR sets up the scaffolding for a new theme codenamed "EUI Amsterdam" (yes, we can change the name later if anyone feels strongly enough about it :).

When setting this up, I basically duplicated the manifest and replaced specific mixin files for the changes I wanted to make on the component-level, and variable changes for everything else. I also went ahead and made a few changes to components to make sure the system worked properly. Those changes include:

• Makes the blue, bluer. Makes the red, redder.
• Bumps up the border-radius from 4px to 7px
• Adjusts the typography:
  • Base font size is now 14px instead of 16px /cc @daveyholler
  • Scales now roughly follow a perfect 4th, which increases the title sizes (I'm aware this might break some stuff, so I'd like to get this merged in so we can pull in the stylesheets and see if anything is not recoverable at that size).
• Removes the borders from panels
• Makes shadows more consistent and smoother

Question

• Is there anything additionally needed to get the new stylesheet included in the final, distributable package?

Checklist

Didn't complete all these since I would consider this a theme that's in beta. But, let me know if I should go ahead and complete all these, or any specific one. I'll make a note in changelog before merging.

• 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

[myasonik]

I'd like to throw out a few a11y related design challenges that might be appropriate to tackle with this revamp:

• Add underline to links (way more words in [Discuss] Adding underlines to links #2446)
• Listen to operating system hints
  • Use @media (prefers-reduced-motion: reduce) to remove all things that move on hover (several things move just a pixel or two [EuiButton] Remove hover state | translateY #2184)
  • Use @media (prefers-color-scheme: dark|light) to auto select a theme
• Better focus states on buttons (Generally, things have a good focus state but it's almost because they are so on brand that it's hard to tell something is in focus if you're not actively navigating a page. Imagine focusing a button then showing someone else the page - ideally, the should be able to tell you which button is focused at a glance. Currently, I think this is pretty difficult with buttons and a few other elements.)
• Remove form controls and force people to use form layouts (Form controls by themselves are accessible and require implementing developers to add their own or use form layouts anyways. We don't want people adding their own patterns around accessible forms because the inconsistency itself is an accessibility issue and, anyways, form layouts allow stripping away much of the visual difference effectively showing just the control.)

[cchaos]

@myasonik I'd like to keep this PR focused on the infrastructure changes needed to accommodate a new theme, one that touches more than just global SASS variables.

But, this is a really great list and I don't want it to get lost in the comments of this PR. Perhaps we should make a Meta ticket for what is to be accomplished with Amsterdam (I'm coming around to it:) and add this stuff to it. @johnbarrierwilson Want to start a Meta issue with the few things you've already outlined and then @myasonik can also add his a11y needs?

The GH issue can also serve as a running list of all the changes that can then be published somewhere for anyone's reference to know the differences in theme.

[ryankeairns]

@johnbarrierwilson Will this be the feature branch you work against as we provide feedback on the design changes?

It may go without saying, but just so everybody involved is on the same page... my thought is that we shouldn't merge this until those discussions and feedback have been taken into account. We could do PRs against this branch or just work from this one exclusively, but let's not merge in any new themes (i.e. let's not expose this to users on the docs site) until we iron things out. Thanks!

[ryankeairns]

^ Related to my last comment, let me know when you're ready for design feedback. I tried this out locally (only skimmed code thus far) and the theme switching worked as expected.

[johnbarrierwilson]

Want to start a Meta issue with the few things you've already outlined, and then @myasonik can also add his a11y needs?

@cchaos @myasonik In this meta issue, I copy/pasted for now. But as more gets added, I'll make sure to keep it organized. Feel free to edit as desired! 😊

Will this be the feature branch you work against as we provide feedback on the design changes?

@ryankeairns I totally understand and agree that we should not expose this to users at this point. At the same time, we would like to start using this new theme in Swiftype properties. If we can get the stylesheet into the build and able to import into our Swiftype projects, I believe it would be a great place to iron things out in context. I do think there's a safe balance we could strike. Would it be acceptable to disable it on the public version of the docs, yet leave the stylesheet in the package build?

Let me know when you're ready for design feedback.

Would love feedback early and often! Not sure the best way to keep up with feedback at this point. Maybe since it's so early and high-level, keep it in Slack (I can create an #eui-amsterdam channel)?

[snide]

Just to give some high level framing and expectation.

1. This should merge to master, not a feature branch, but you can branch off of this branch to do reviewable chunking (see EuiDataGrid's PR for an example of this). Doing something like this means we'll need to keep it up to date and we can't forget it exists. That means it needs to go through proper review before it goes in, and then we need to consider it on every PR we do later against EUI. @ryankeairns can handle the overall review and I'd suggest you two figure out a way to do that in a regular cadence. I would expect this process to take weeks, not days, because there is a lot of change and has a high impact. That said, it does not need to be complete on merge. It can iteratively change over time.
2. There is no worry about this being public, but it should not be in use in production until we're happy with it (for Kibana, this will be no earlier then as a beta in a late 7.x minor). It can be delivered in EUI's dist though whenever we merge here. This gives us time to test it anywhere we want over the next six months in a safe, easy way.
3. The scope needs to be stylistic only. We're not touching the dom here. This is not an accessibility project. Those issues can be tackled in parallel against master.

[ryankeairns]

@johnbarrierwilson Understood and +1 to testing within context. I'll give it a more thorough design review and send you some feedback so we can get this first pass in.

After that, we can do the contextual reviews (Swiftype, Kibana, Cloud) and keep iterating on the various sections that are being touched. Sounds like a plan!

[cchaos]

@johnbarrierwilson I'm doing an initial review purely on the architecture. I'm sure you've noticed that we're not quite setup currently for being perfectly theme-able. And as I've fussed around a bit in the code and drawing out the current architecture in Whimsical, I think we will need to take this even further and not worry about introducing breaking changes to the file/import structure. (So long as we are mindful of how each breaking changes PR will increase the EUI versioning and try to combine as many into one PR as possible in the beginning).

Here's my diagram of how the file system is currently architected, how this PR tries to utilize the current architecture but can't quite manipulate it efficiently, and how we should be architected to allow for easier theming.

Your change to the EuiPanel mixin is a perfect opportunity to see how other components rely on that particular mixin which is established in the component file instead of a global file. You can see in the diagram that card's are reliant on this mixin but unless you then also override all of the card imports, it still uses the old euiPanel() mixin.

What does this all mean?

Essentially, we (all maintainers of EUI) need to be cognizant of deciding when to create global and when to create component-only mixins. I'm not advocating that every single component mixin needs to be global. But we need to consider if it most likely will get re-used. Or if we end up re-using one, that we move it to the globals (and probably rename it).

I'm creating a PR that will showcase the euiPanel() example

[ryankeairns]

@cchaos I really like the clean cut rule at the end of your explanation... if you re-use a mixin from another component, then that mixin should be moved to global.

[johnbarrierwilson]

Agreed! In fact, I think that clear cut rule renders this PR invalid. The other work that was included in this PR is easily replicable in another PR with proper architecture.

[cchaos]

My comment was not meant to say that this PR was wrong, but that it just needs to architecture tweaks. It's totally valid to keep this PR open and continue to update it.

[johnbarrierwilson]

Not taken that way all! :) I just think that your direction is different enough to warrant closing this since the architecture work is more in-depth than the style changes I made. So, from my perspective, it's more simple, easier and different enough to move to a different PR.

[cchaos]

Sure thing, whatever works best for you!