#772 Improve Browser Support for new components#816
Open
NoelDeMartin wants to merge 6 commits into
Open
Conversation
- Update dependencies range to dev prereleases - Move tailwindcss to devDependencies (reset CSS is bundled in dist) - Remove React dependencies (no longer needed for Storybook 10)
fa69b34 to
5bb90bd
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves cross-browser compatibility for Solid UI components and build outputs by removing reliance on newer/less-supported platform features (Popover API, :state() selectors, direct FS copy/write steps) and consolidating build-time Babel behavior into solidos-toolkit.
Changes:
- Replace local Vite Babel plugin with
solidos-toolkit/viteand centralize Lit decorator path configuration. - Improve runtime browser support by updating component behavior (Combobox popup implementation, Dialog
showModal()guard,:state()→data-state-*). - Make build artifacts more bundler-native via
emitFile()(CSS d.ts + CDN legacy alias generation).
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.mts | Switch Babel integration to solidos-toolkit/vite and pass Lit decorator paths. |
| vite-config/styles.ts | Emit theme.css.d.ts via Rollup asset emission instead of writing to disk. |
| vite-config/components.ts | Introduce litDecoratorPaths configuration used by Babel plugin. |
| vite-config/cdn.ts | Use solidos-toolkit/vite Babel + change legacy alias generation from FS copy to bundle emission. |
| vite-config/babel.ts | Remove the repo-local Babel plugin implementation. |
| tsconfig.json | Include .storybook/**/* in TypeScript compilation scope. |
| src/lib/components/web-component/WebComponent.ts | Replace ElementInternals state toggling with data-state-* attributes for broader CSS support. |
| src/lib/components/traits/InputTrait.ts | Add stable label id for improved ARIA wiring. |
| src/components/dialog/Dialog.ts | Guard showModal() usage for browsers without support. |
| src/components/dialog/Dialog.styles.css | Remove closed-dialog display:none override. |
| src/components/dialog-provider/DialogProvider.ts | More reliable slot lookup via querySelector('slot'). |
| src/components/combobox/Combobox.ts | Replace Popover API usage with wa-popup and implement keyboard/mouse open/selection logic + ARIA. |
| src/components/combobox/Combobox.styles.css | Style wa-popup and update listbox/active option styling. |
| src/components/avatar/Avatar.styles.css | Update state styling to data-state-* selectors. |
| src/components/account/Account.styles.css | Update state styling to data-state-* selectors. |
| package.json | Dependency adjustments (introduce solidos-toolkit, move/update versions). |
| package-lock.json | Lockfile updates reflecting dependency and transitive changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5bb90bd to
b53f40b
Compare
- Reuse babel config from solidos-toolkit - Replace closeBundle usage with generateBundle emits - Fix storybook TS config - Use bundler module resolution
Only 0.01% of our target browsers don't support it (KaiOS and Opera Mini), so it's not worth adding any polyfills. However, these changes make it at least usable on these browsers given that a <dialog> will be rendered as a plain unknown element instead of crashing.
Prompts: - Looking at https://github.com/shoelace-style/webawesome/blob/next/packages/webawesome/src/components/select/select.ts, can you update the Combobox component to be accessible? Also, replace the popup and anchor CSS positioning by using Web Awesome's popup instead (wa-popup). - Can you review the ARIA guidelines for our current implementation? https://www.w3.org/WAI/ARIA/apg/patterns/combobox/ AI Summary of the work: - Replaced native popover and CSS anchor positioning with wa-popup - Added combobox ARIA roles, states, and keyboard navigation - Aligned aria-selected, labeling, and keyboard behavior with APG guidelines - Close popup on outside click or focus, Escape, Tab, and empty filter results - Added label ids in InputTrait for aria-labelledby Co-Authored-By: Cursor <cursoragent@cursor.com>
b53f40b to
d738e7d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR aligns the features used in the new components with our target browsers, in particular:
:state()usage.popoverusage.<dialog>usage. It is supported by almost all our target browsers, but it should be "usable" for the 0.01% that don't.It also improves a11y for the Combobox component, and cleans up some Vite and package.json configs.