Skip to content

#769 Update account components in header#775

Open
NoelDeMartin wants to merge 13 commits into
SolidOS:stagingfrom
NoelDeMartin:769-design-system-header-account
Open

#769 Update account components in header#775
NoelDeMartin wants to merge 13 commits into
SolidOS:stagingfrom
NoelDeMartin:769-design-system-header-account

Conversation

@NoelDeMartin
Copy link
Copy Markdown
Member

@NoelDeMartin NoelDeMartin commented May 29, 2026

This PR introduces the new web components we've been discussing over the last few weeks.

Here's some things to note:

This has been implemented as a swappable interface to support other environments easily, and particularly in order to eventually converge on an interoperable system for all Web Component libraries in the Solid ecosystem
NoelDeMartin and others added 8 commits May 29, 2026 10:45
Prompt: We're using the tailwindcss preflight resets in @src/primitives/styles/common.styles.css, but they have an overly aggressive selector that resets the margin and padding of any element. This is a problem because styling web component elements using :host in their own styles take less precedence in the situations were nested components are rendered. Use the patch-package utility to modify the built-in preflight and remove the padding and margin resets in the * selector. Instead, create a new rule that only resets padding and margin for built-in elements.

Summary of Changes:

1. Modified Preflight Styles: Updated preflight.css to:
  • Remove  margin: 0;  and  padding: 0;  from the universal  *  selector rule.
  • Create a new rule resetting margins and paddings only for standard built-in HTML tags and pseudo-elements (such as  ::after ,  ::before ,  ::backdrop ,
  ::file-selector-button ).
2. Generated Patch: Ran  npx patch-package tailwindcss  to generate the patch file at tailwindcss+4.3.0.patch.
3. Staged and Verified:
  • Verified that  npm run typecheck && npm run build  and  npm run test  compile and pass without issues.
  • Staged the patch file along with the formatting tweak in package.json.

Co-Authored-By: Antigravity CLI (Gemini 3.5 Flash) <bot@antigravity.local>
@NoelDeMartin NoelDeMartin force-pushed the 769-design-system-header-account branch from 7a2712a to b56e25a Compare May 29, 2026 08:46
@timea-solid
Copy link
Copy Markdown
Member

timea-solid commented May 29, 2026

I would expect each component to have a Readme as well, for showcasing how to use it. (not just in Storybook). I fail to see how I would change the default sign up link.
Also, should each component have a test?

I see you removed the storybook stories we had. I would still need the forms stuff at least in - and once I move out the rdf forms part of the code, than delete it.

@NoelDeMartin
Copy link
Copy Markdown
Member Author

I haven't thought much about documentation yet, but the Storybooks already generate a Docs section with the current config (I updated my fork's deployment again: https://noeldemartin.github.io/solid-ui). Maybe that could be a follow-up task?

Regarding the storybooks I deleted, I thought all the components will need to be migrated to the new conventions so I preferred to clean up the files so that the storybook is up to date (and avoid confusing designers or anyone else who looks at the storybook). Keep in mind that "deleting" something in git is not really deleting it, we always have the history and can always restore it or see what it contained. Do you still think we should keep some of the old stories around?

@timea-solid
Copy link
Copy Markdown
Member

Very nice and clean commit history. I also like the AI related commit. Nicely done. I will use it as an example in our guidelines.

@NoelDeMartin
Copy link
Copy Markdown
Member Author

I fail to see how I would change the default sign up link.

By the way, this can be configured in the <solid-ui-provider> component, which is not in the storybook because it's just a logic component (it doesn't have any UI). Instead of taking the signupUrl directly, it should probably take a config option, though. Or maybe we want to set the configuration using a global object in window or something. That's something we should decide eventually, and document it, but for now I think it's ok to have it there as we keep building the design system.

Also, should each component have a test?

Well, not every component should have a test, but certainly each component can have a test. Ideally, I like to avoid redundant tests, for example I'm not much of a fan of writing tests like this one we have for the Header:

  it('is defined as a custom element', () => {
    const defined = customElements.get('solid-ui-header')
    expect(defined).toBe(Header)
  })

I prefer to test most UI using E2E tests that actually use the application for real. Still, I do agree with having some component unit tests for testing edge cases and so on.

Looking at the components I introduce in this PR, I don't think we need any specific test. Is there any that you had in mind?

I do agree that we're lacking the E2E tests, though. I have created an issue to look into this #777.

@timea-solid
Copy link
Copy Markdown
Member

Removing the storybook stories is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace User Menu and Log In / Sign Up buttons with Design System components

2 participants