eui
eui copied to clipboard
[EuiTabs] Remove `xl` size
Description
The purpose of this task is to remove the xl size in EuiTabs.
It's been discussed in https://github.com/elastic/eui/discussions/8128. Two reasons for this request are:
- Subjectively, it is an atypical font size for tabs (see below). The guidelines say to only use for page titles only, but I don't believe this is a justifiable recommendation.
- Objectively, we don't use it in Kibana. Search for usage with this regex
<EuiTabs[ \w=\{\}>\(\)\n;".-:]* size="xl".
[!NOTE] This is a breaking change with low impact. We can suggest using
las the recommended fix.
Acceptance criteria
- [x]
xlsize is removed fromEuiTabs
Sorry @ryankeairns, we missed this one. It wasn't added to our team board automatically for some reason. It's here now, we'll take a look.
The code changes are small. There are no usages in Kibana and Cloud UI (used the regexp from the description). We should treat it as a breaking change regardless (also to account for external contributors). I put my preliminary estimation.
Thanks all.
Another place these can surface is through EuiPageTemplate (docs) when no heading is provided. In any case, my reasons stand :) ... and I will address feedback that comes as a result of the change.
I assume we'll need to make sure this is updated in Figma after we make this change as well.
Labeled this as a breaking change -- we should consider pairing this with other breaking changes in a planned release. If we do that, we should plan for a breaking change release and give folks extra time to test in Kibana.
I assume we'll need to make sure this is updated in Figma after we make this change as well.
Created an issue on our EUI Design work board and will have this ready to roll in conjunction with the component change.
@ryankeairns We have to account for direct EuiPageHeader and KibanaPageTemplate.Header usages (that use EuiPageHeader underneath). Specifically, those that pass tabs and NOT pageTitle at the same time. They fall into this condition:
https://github.com/elastic/eui/blob/803bdb959de54eff4aa96789a3644885370bd39d/packages/eui/src/components/page/page_header/page_header_content.tsx#L273-L326
In theory, there will be no code changes on Kibana side but there could be visual changes (and there could be snapshot changes). I have a PR open just for removing the size: https://github.com/elastic/eui/pull/8762
Do we want to proceed or do we want to pause the change?
Thanks @weronikaolejniczak
This is helpful. With this information, I will communicate the proposed change to the EUI ambassadors (meeting next week) so that they are aware and have the opportunity to voice any concerns.
So, yes, hold for just a bit, and I will knock on some doors.
@ryankeairns just before falling asleep I realized that I read the condition wrong 😅 The affected cases would be if there are tabs passed but NO pageTitle. I searched in Kibana and Cloud, and I didn't find any EuiPageHeader or KibanaPageTemplate.Header that pass tabs and not pageTitle (so they all should use the l size). That being said, might be worth to double-check by someone and raise this change on the EUI ambassadors meeting.
@weronikaolejniczak Should we move this to blocked until Ryan has a chance to talk to the ambassadors?
@JasonStoltz there are no usages of EuiPageHeader or KibanaPageTemplate.Header, so I think we're fine to proceed but I can put it in Blocked for now anyway.
@ryankeairns just before falling asleep I realized that I read the condition wrong 😅 The affected cases would be if there are
tabspassed but NOpageTitle. I searched in Kibana and Cloud, and I didn't find anyEuiPageHeaderorKibanaPageTemplate.Headerthat passtabsand notpageTitle(so they all should use thelsize). That being said, might be worth to double-check by someone and raise this change on the EUI ambassadors meeting.
Got it. I also could not find any use cases of this. I have sent a message in #ux-wg-patterns and will reiterate the point on Thursday, in person.
Thanks for following through on this. I received 0 feedback and interpret that to mean there would be little to no impact.