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.

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

[Feature Request]: Add <Layer> to all Carbon containers that change the background-color to $layer #17943

Open
1 task done
wkeese opened this issue Nov 1, 2024 · 6 comments
Labels
component: modal component: tabs component: tile proposal: open This request has gone through triaging. We're determining whether we take this on or not. type: enhancement 💡

Comments

@wkeese
Copy link
Contributor

wkeese commented Nov 1, 2024

The problem

Whenever we use a component like <Tile> we need to wrap <Layer> around the Tile’s contents:

<Tile>
    <Layer>
        <TextInput .../>
        ...
    </Layer>
</Tile>

This is because _tile.scss sets

background-color: $layer;

... and so you need the <Layer> to make child <TextInput> etc. have a background that contrasts with the Tile's background.

Otherwise it ends up looking like this:

Image

The solution

Make Tile, ExpandableTile, and Tabs add Layer when they change the background color.

Also need to update @carbon/ibm-products' Tearsheet and TearsheetNarrow, but that's a different bugdb.

Examples

Tile

Application/PAL

IKC

Business priority

None

Available extra resources

No response

Code of Conduct

Copy link
Contributor

github-actions bot commented Nov 1, 2024

Thank you for submitting a feature request. Your proposal is open and will soon be triaged by the Carbon team.

If your proposal is accepted and the Carbon team has bandwidth they will take on the issue, or else request you or other volunteers from the community to work on this issue.

@kennylam
Copy link
Member

Hi @wkeese, can you elaborate a bit more about this, i.e., would it provide any additional functionality apart from saving users from typing out <Layer /> tags? I'm wondering how this would affect users who do not want to include Layer in their markup.

@wkeese
Copy link
Contributor Author

wkeese commented Nov 25, 2024

Hi @wkeese, can you elaborate a bit more about this, i.e., would it provide any additional functionality apart from saving users from typing out <Layer /> tags?

It's like asking: why does Carbon's _tile.scss setting background-color: $layer? Does it provide any additional functionality apart from saving users from typing out background: $layer?

I would like Carbon to just work correctly out-of-the-box, rather than users having to tweak it.

In other words, the answer to your question is "no", but it's a strange question, because everything @carbon/react does is to save users from typing out JS or markup. This is no different.

I'm wondering how this would affect users who do not want to include Layer in their markup.

You're asking how users can get a non-standard coloring for the TextInput etc. components. You either use <Layer> or <Theme> or (as a last resort) just override Carbon's CSS directly.

But why are those users contradicting IBM's designated styling for TextInputs?

In other words, Carbon should be optimized for the common case.


Basically, background-color: $layer should always be paired with <Layer>. That's the point of this ticket.

@sstrubberg
Copy link
Member

Hey @wkeese, would you be open to attending Carbon Office Hours? This change could have wider implications across the system and we want to make sure we understand this use-case. Thanks!

@sstrubberg sstrubberg moved this from Triage to Later 🧊 in Roadmap Jan 6, 2025
@tay1orjones
Copy link
Member

Yeah to share some more context, adding Layer to ComposedModal made sense because Modal always needs that layer bump.

For Tile and Tabs there are potential use cases where a Layer is not needed or desired. If we force a layer on these components, there's no way to opt out other than adding 2 more Layer to "loop" back around to the base layer.

If the office hours discussion still lands on forcing a Layer in these components, we need to consider it's probably a breaking change. For ComposedModal we already had styles in place that made sure layer styles were correct for all the inputs, etc. but Tile and Tabs don't have that. Consumers who have already configured a Layer with these would be impacted by a Layer being added to the components.

@wkeese
Copy link
Contributor Author

wkeese commented Jan 7, 2025

Hey @wkeese, would you be open to attending Carbon Office Hours?

That's really difficult for me, it's literally the middle of the night here.

For Tile and Tabs there are potential use cases where a Layer is not needed or desired. If we force a layer on these components, there's no way to opt out other than adding 2 more Layer to "loop" back around to the base layer.

Can you give some practical examples? The devil is in the details.

I saw in https://react.carbondesignsystem.com/?path=/story/components-tile--expandable-with-interactive where the input has the same background as the tile, but (as I said above) it looks like a mistake to me, not a feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: modal component: tabs component: tile proposal: open This request has gone through triaging. We're determining whether we take this on or not. type: enhancement 💡
Projects
Status: Later 🧊
Development

No branches or pull requests

4 participants