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

[ic-theme dx] consider replacing 'foreground selection' logic of slotted components with CSS Shadow Parts #161

Closed
MI6-27 opened this issue Dec 13, 2022 · 1 comment
Assignees
Labels
card component The generic component, includes both the web component and the React component component: ic-theme The generic component, includes both the web component and the React component dx Related to developers' experience footer component The generic component, includes both the web component and the React component hero component The generic component, includes both the web component and the React component link component The generic component, includes both the web component and the React component spike This issue needs to be researched and explored before determining a course of action

Comments

@MI6-27
Copy link
Contributor

MI6-27 commented Dec 13, 2022

Summary

Currently, slotted components choose their 'foreground appearance' based off some non-trivial getThemeFromContext logic in utils/helpers.ts on line 154. While this reduces the burden on developers implementing themed and slotted components (in any framework), it is not clean code to maintain.

💬 Description

Take the below example. You will note that a custom theme of a yellow colour rgb(255, 201, 60) is used. The button used will be a 'dark foreground' variant (it will be black background and white text). Yet, the implementer has not had to set any props or configure that behaviour. The benefit of this, in the very edge case, that a theme changes during a render, relevant child components will choose an accurate appearance.

<ic-theme color="rgb(255, 201, 60)"></ic-theme>
<ic-hero
  heading="Hero heading"
>
  <div slot="interaction">
    <ic-text-field
      label="Filter display"
      hide-label
    ></ic-text-field>
    <ic-button variant="primary"
      >Filter</ic-button
    >
  </div>
</ic-hero>

However, this requires some non-trivial configuration in src/utils/constants.ts and code to be included in relevant 'clever' components. See lines 131 and 199 of ic-button.tsx.

💰 Use value

CSS Shadow Parts allow component authors to expose 'parts' of their DOM to implementers and allow CSS styling (almost all properties work) on those expose DOM elements.

I'm not suggesting we expose/document this to implementers, but it might streamline/clean up our implementation of themeing.

In future, this could also be exposed to ui-kit implementers, like the IONIC framework does, to allow further customisation of components (or parts thereof).

You can see this in action. The example below will add a yellow background to all ic-cards that are slotted into ic-heros, but only when the ic-hero is 'dark foreground'.

/* helpers.css */
ic-hero.dark ic-card::part(container) {
  background:yellow
}
// Line 170-ish of ic-card.tsx
<Component
  // ...
  disabled={disabled ? true : null} 
  part="container"
  {...attrs}
>

Additional info

Read more:

@MI6-27 MI6-27 added type: enhancement dx Related to developers' experience footer component The generic component, includes both the web component and the React component card component The generic component, includes both the web component and the React component hero component The generic component, includes both the web component and the React component component: ic-theme The generic component, includes both the web component and the React component link component The generic component, includes both the web component and the React component labels Dec 13, 2022
@ASM995 ASM995 added the spike This issue needs to be researched and explored before determining a course of action label Jan 19, 2023
@ASM995 ASM995 moved this from In Refinement to Ready for dev in Intelligence Community Design System Jan 19, 2023
@GCHQ-Developer-112 GCHQ-Developer-112 self-assigned this Feb 22, 2023
@GCHQ-Developer-112 GCHQ-Developer-112 moved this from Ready for dev to Dev In Progress in Intelligence Community Design System Feb 22, 2023
@GCHQ-Developer-112
Copy link
Contributor

My understanding of this ticket is that this is a nice-to-have addition. We already have logic that manages the theme of the foundational components. As part of this ticket, I explored using the shadow parts to replace this logic. Shadow parts in themselves are very useful, especially when we are trying to repurpose components.

For example, I've been exploring repurposing <ic-button> in our new <ic-menu-button> component. These components share similar functionality and so using shadows parts prevents duplicated code. <ic-menu-button> differs in its styling so I've been able to export the <button> in <ic-button> by adding the attribute part="button". In ic-menu-button.tsx, I have used exportparts="button" to be able to target the <button> in <ic-button> and re-style the button as needed in ic-menu-button.css.

Although, I've been able to see the benefits of using shadow parts elsewhere. I've been unable to see the benefits of using shadow parts as part of our theme changing logic. This is because unlike in the example above, the background colours used as a result of a theme change are never consistent. Our theme logic handles the inconsistency well, adapting and changing based on the colour based through <ic-theme>. With that being said, I will be closing this ticket as I do not see a way forward with this ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
card component The generic component, includes both the web component and the React component component: ic-theme The generic component, includes both the web component and the React component dx Related to developers' experience footer component The generic component, includes both the web component and the React component hero component The generic component, includes both the web component and the React component link component The generic component, includes both the web component and the React component spike This issue needs to be researched and explored before determining a course of action
Projects
Development

No branches or pull requests

3 participants