fix(Tabs & Tabs extended with media): adjust the layout to align with the defined grid

[mwanberg]

Related Ticket(s)

[Tabs & Tabs extended with media]: Not on grid

Description

1. Tabs extended media: Contents of the component are not aligned to Carbon grid.

Current Canary web components:

Expecting:

Updated:

2. For Tabs extended, the individual tabs should be 2 columns wide.

Current:

Expecting:

Updated:

Notes

As of this first pass, there are still some differences between the specs and the results, notably:

1. Tabs Extended with Media
   • The tab contents' image and text are not aligned to the grid
   • Some of the vertical margins of the tab contents are different
2. Tabs Extended
   • The tabs' text is not bottom aligned when they have multiple lines
   • The tab contents' text and cta block have some left misalignment due to inherited styles from the component it's using, Content Block Media

I am unsure if the scope of this ticket includes addressing the above.

[ibmdotcom-bot]

Deploy preview created for package React:
https://ibmdotcom-react.s3.us-south.cloud-object-storage.appdomain.cloud/deploy-previews/8394/index.html

Built with commit: 190af76e7ac97fd65b0268d14b31041ef9ec1d33

[ibmdotcom-bot]

Deploy preview created for package Web Components HTML Test Application:
https://ibmdotcom-web-components-html-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8394/index.html

Built with commit: 190af76e7ac97fd65b0268d14b31041ef9ec1d33

[ibmdotcom-bot]

Deploy preview created for package Web Components (React wrapper):
https://ibmdotcom-web-components-react.s3-web.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8394/index.html

Built with commit: 190af76e7ac97fd65b0268d14b31041ef9ec1d33

[ibmdotcom-bot]

Deploy preview created for package Web Components:
https://ibmdotcom-webcomponents.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8394/index.html

Built with commit: 190af76e7ac97fd65b0268d14b31041ef9ec1d33

[ibmdotcom-bot]

Deploy preview created for package NextJS Test Application:
https://ibmdotcom-nextjs-test-upstream.s3.us-east.cloud-object-storage.appdomain.cloud/deploy-previews/8394/index.html

Built with commit: 190af76e7ac97fd65b0268d14b31041ef9ec1d33

[proeung]

@mwanberg Thanks for opening this draft PR. I just looked at the WC deploy preview and I noticed that there's some 16px spacing between each tab, which I don't believe is part of the design specs are in the ticket that Shixie has outlined here (#7815) (see attached). Can you take a look?

Also, do you mind outlining the style targeting issue that you're having? You should be able to target elements within the shadow root by referencing the slotted items.

[mwanberg]

@proeung Thanks for pointing out the tab width issue - that should be resolved now.

Here is what I am encountering with the tab / accordion contents styling:

1. For Tabs Extended, the content of the tab (accordion in mobile) is a separate component, dds-content-item, which has its own styling.
2. For Tabs Extended with Media, the content of the tab is dds-content-item-horizontal-media, which also has its own styling.

Am I mistaken in that it will be very challenging to affect the styling of those elements without extending them in the Tabs Extended / with Media?

Example of dds-content-item margins on Tabs Extended:

Example of content-item-horizontal grid styles on Tabs Extended with Media:

[proeung]

@mwanberg I see what you're trying to do here now. If you want to remove the margin around the dds-content-item component within the dds-tab, you can target it through the slotted return. Something like :host(dds-content-block-media-content) ::slotted([slot='dds-content-item']).

In this case, where you want the text in the tab content to align with the text of the tab itself, it might be better to remove the extra padding on :host(dds-tabs-extended) .bx--tab-content (see attached).

[mwanberg]

@mwanberg I see what you're trying to do here now. If you want to remove the margin around the dds-content-item component within the dds-tab, you can target it through the slotted return. Something like :host(dds-content-block-media-content) ::slotted([slot='dds-content-item']).

Do you mean I should target it in the Tabs Extended styles or in the Content Block Media styles? I attempted to put that selector in the Tabs Extended styles and got no results. Perhaps I am misunderstanding your suggestion.

In this case, where you want the text in the tab content to align with the text of the tab itself, it might be better to remove the extra padding on :host(dds-tabs-extended) .bx--tab-content (see attached).

I actually do not know how it should be aligned. In the Tabs Extended specs, nothing is defined for the desktop width contents. However, the mobile spec does have contents (see page 14), and the text there doesn't have the added margins (unlike in the original Content Block Media component), so I am assuming that it would be the same for the desktop version (to have no margins around the text).

For both the Tabs Extended and Tabs Extended with Media, the specs have them using what are essentially Content Block Media and Content Item Horizontal (with Media) respectively, but with somewhat different grid alignments and margins than the originals.

Do you think it is within scope of this ticket to attempt to align the tab contents (of both or either Tabs Extended or Tabs Extended with Media) to the original specs?

Are these tabs components supposed to be agnostic in terms of what goes in the tab contents? Or are those tab contents supposed to for sure look a certain way? If the former, then I would guess that updates are not needed. If the latter, then it becomes a scope and effort question.

I will update the description of this PR to reflect the original requests, what I've updated, and what might remain.

[proeung]

Do you think it is within scope of this ticket to attempt to align the tab contents (of both or either Tabs Extended or Tabs Extended with Media) to the original specs?

Personally, I don't think it's out of the scope of (#7815) to ensure that both Tabs Extended or Tabs Extended with Media are aligned to the design specs, however, I'll defer to @oliviaflory and @kennylam on this.

@kennylam Also, this feels like it's related to our conversation about reducing the number of variations that might lead to the extension of components in order to target specific styling.

[andy-blum]

Heads up: there might be conflicts with this PR: #8365

[oliviaflory]

Hi @mwanberg adding my perspective, and I hope this helps to clarify a few things!

I actually do not know how it should be aligned. In the Tabs Extended specs, nothing is defined for the desktop width contents. However, the mobile spec does have contents (see page 14), and the text there doesn't have the added margins (unlike in the original Content Block Media component), so I am assuming that it would be the same for the desktop version (to have no margins around the text).

I'm very sorry that this was confusing! Tabs extended was originally designed as Tabs extended with media and we decided to build the tabs out separately so Tabs extended could take on many forms of content.

You are correct in assuming the tabs extended content should not have any margins around the text in the desktop version. Image example below, and I will make a new issue for us to update the design specs to make this more clear for future reference!

For both the Tabs Extended and Tabs Extended with Media, the specs have them using what are essentially Content Block Media and Content Item Horizontal (with Media) respectively, but with somewhat different grid alignments and margins than the originals.

Unfortunately it looks like we updated the Content item horizontal with media spec when the image is left aligned and did not go into Tabs extended with media to update the visual (so you are seeing an old spec and that's why it doesn't look quite right!) 😞 I will make an issue to address this as well.

The intent is that the Tabs extended are agnostic. IMO Tabs extended with media is an example of what type of content you could place into the tabs (Content block with media), the content should adhere to how it was designed on the grid. Below is the latest content block with media (left) in the Tabs content area. I assume it would just "plop" in and sit nicely 😅

I personally agree with @proeung that ensuring both tabs, tabs extended, and the content within the tabs and tabs extended are properly on the grid is within the scope of the original issue. However that might make the issue more than 1 story point(?)

[mwanberg]

I personally agree with @proeung that ensuring both tabs, tabs extended, and the content within the tabs and tabs extended are properly on the grid is within the scope of the original issue. However that might make the issue more than 1 story point(?)

Thank you so much for replying and providing some insight, @oliviaflory! That is very helpful.

Two followup questions:

1. For updating the contents of the tabs, should we do that in their original components (e.g. Content Item Horizontal (with Media)) or extend those so we can update the styling within Tabs Extended / Tabs Extended with Media?
2. If we need to update the story points, who should do that and to how much?

[oliviaflory]

1. For updating the contents of the tabs, should we do that in their original components (e.g. Content Item Horizontal (with Media)) or extend those so we can update the styling within Tabs Extended / Tabs Extended with Media?

I believe we'd want to use the Content item horizontal (not extend or change the component to accommodate Tabs extended). But I am double checking with the DDS engineers.

2. If we need to update the story points, who should do that and to how much?

Storypoints are usually estimated and set by the developers, so I'd leave it up to you to let us know how much time this took and we can update. It helps us and you document how much time and effort is taken within a sprint

[mwanberg]

I believe we'd want to use the Content item horizontal (not extend or change the component to accommodate Tabs extended). But I am double checking with the DDS engineers.

@oliviaflory great! Please let me know what the consensus is so I know how to finish this PR. I'd like to get it in for this sprint, though I know it's not a super high priority one.

[proeung]

@mwanberg This PR (#8365) has been merged, please move forward with addressing the merge conflict in this PR.

As for your first question in this comment (#8394 (comment)), please reach out to one of the DPO engineers (@IgnacioBecerra , @ariellalgilmore , or @emyarod ), so that we can move this work forward. Thanks!

[mwanberg]

This PR is now ready for review. As discussed in this Slack thread, we will not be tackling the layout of the tab contents in this PR. A followup ticket for the contents is here: #8470

[proeung]

LGTM from a code perspective!

I'll defer to @oliviaflory on the design QA side to ensure that the changes are expected. https://percy.io/538fc19a/Carbon-for-IBM.com-Web-Components/builds/16409602/changed/923734665?browser=chrome&browser_ids=20&subcategories=unreviewed%2Cchanges_requested&viewLayout=overlay&viewMode=original&width=1280&widths=320%2C1280

[oliviaflory]

@mwanberg the shift in the tabs story content does feel like a regression (outlined in pink). Can you remind me if something was changed in the content area or if the 16px shift to the right is due to something in the content item?

In the current tabs story the content is aligned correctly, with the type aligning to the column.

---

For tabs extended with media the Percy difs are as expected 👍

[mwanberg]

@oliviaflory Hm, if I remove the side padding from Tabs Extended tab content, then that will make the Tabs Extended with Media tab content become misaligned. However, I think that might be better, because it finishes making the tab content area agnostic in terms of what components get plopped in.

So I will make that change and remove the added padding, but you will now see a visual diff change in Tabs Extended with Media, in that content area.

The unnecessary padding, which I am now removing, is outlined in pink. It will now be up to the components that get put in to add their own padding as necessary. Does that sound ok?

[oliviaflory]

@mwanberg Thanks for explaining! The previews are still building, but I will check back on Percy / Deploy previews. I think this makes sense..just about weighing all the repercussions 👍

[oliviaflory]

Thanks for the update @mwanberg