Initial pre-release - Carbon v11 styles

theetrain commented 8 months ago

Closes #1636, closes #1709, closes #1886
Relates to: #1872, #1629

This is the initial prerelease for carbon-components-svelte featuring:

Carbon v11 styles (from @carbon/styles@1.46.0)
borrowed adaptations from #1709 (thank you @gregorw)

Questions

Should we support deprecated property values such as ContentSwitcher's size="xl"? I think we shouldn't.
Should default sizes be explicit strings such as size="md" instead of "size=undefined"? It may be better to use explicit values when writing a form generator, but they can always return size=null I suppose. Search.svelte supports size="md"; maybe this can be addressed as part of #1621

Before merging todos

[x] upgrade @carbon/styles to latest
[x] remove DatePicker size="xl" option
[x] test ExpandableTile docs
[x] Test Toggle and ToggleSkeleton
[x] Test TreeView docs
[x] Restore Truncate component
[x] Regenerate CSS files
- [x] use --cds class prefix from flagship package instead of --bx
- [x] create truncate classes from mixins
- [x] include CSS in docs
[x] Update Theme component to use new data- attributes
[x] Ensure docs render nicely using v11 styles

Deferred todos:

[ ] Components with glaring issues (to be resolved later)
- [ ] CodeSnippet component
- [ ] Black 'search' component
- [ ] Notification CSS
- [ ] Tile
[x] Test in a new local SvelteKit project to ensure modules can link
[ ] Identify which components are 'v11 ready' and which aren't in #1629

After merging todos

[x] "squash and merge" this PR onto the next/v11-styles branch (and subsequent contributions); use 'breaking changes' message below in commit body
[x] Rebase next/carbon-v11 onto master to get latest fixes and features
[x] with next/carbon-v11 checked out, deploy package as 1.0.0-alpha.0 in package.json (v1.0.0-alpha.0 in git)
[ ] Include install and contribution instructions in release notes; update related issues and discussions; (remember to mention using npm)
[ ] Consider deploying docs based on changes in the next/carbon-v11 branch

👀 The following can go into the squashed commit message:

Breaking changes

Overall, this is a major style change the will impact the appearance and features of many components. Use caution when upgrading and test your applications.

Components

Button has new prop values for size and kind
Theme follows v11 conventions: g80 theme isn't supported, toggled themes adjust data-carbon-theme attribute in <html> tag (for now, tokens use bx prefix, but that may change)
Tabs has a contained prop instead of type
ContentSwitcher size prop no longer supports size="xl"; md is the new default
MultiSelect no longer supports xl size
OverflowMenu no longer supports xl size
Search no longer supports xl size
TreeView no longer supports compact size
UIShell has a different appearance per theme

CSS

Several class names have been changed due to the v11 overhaul. If you're targeting or overriding component classes, be sure to test your code
Many tokens and CSS variables have been renamed. Details: https://carbondesignsystem.com/migrating/guide/develop
Themes are applied to the <html> element as data-carbon-theme="g10" instead of theme="g10"
The g80 theme no longer exists

General

Codebase uses npm instead of yarn

gregorw commented 8 months ago

@theetrain I’m glad to see activity towards a v11 styles adoption. What I observe is that this PR is already large and it’s growing. Decisions such as “switch to npm” should be discussed in entirely separate PRs and not mixed up with v11 styles. Also, I would abstain from switching all class prefixes from bx to cds. This introduces a lot of noise or churn within the PR which is unnecessary. I’d rather discuss this at a later stage in a separate PR.

What’s the minimal change that we can merge onto next/v11-styles branch? My worry is that this PR will be open for too long or introduce too many changes. Should we rather merge one component at a time? Let’s take small (but steady) steps: #1886

theetrain commented 8 months ago

Thanks for chiming in @gregorw; this week, I am working full-time on getting v11 styles in a prerelease and my goal is to get as much done before Friday so that contributors could have an easier time focusing on the components. You're right though, it's better to release early and often so I'll wrap this up today as best I can.

Decisions such as “switch to npm” should be discussed in entirely separate PRs and not mixed up with v11 styles.

Absolutely, though in this case I had hit a wall when running the docs due to cryptic node-gyp install errors. Since we employ npm provenance (#1842), I figured I could side-step the node-gyp errors and switch to npm for the next/v11-styles branch; we'll be switching to npm soon enough anyway.

Also, I would abstain from switching all class prefixes from bx to cds.

I'll switch back to bx then, with the intention to switch to cds in a future PR before completing the v11 style integration.

theetrain commented 8 months ago

Hi @metonym; please have a look at Gregor's comment above. In the PR body, have a read over the questions and 'after merging todos' to see if we can comfortably start a prerelease with these changes.

I understand the PR is huge with 188 file changes. After merging, we and the community can review components in #1629 until full coverage is attained; and it should be straightforward to review one component at a time given the new theme files are made and the theme toggle is functioning.

After your approval, and if you prefer, I can help merge and then rebase the next/v11-styles branch, and deploy the prerelease.

metonym commented 8 months ago

Nice work @theetrain and @gregorw! It's shaping up very nicely.

Should we support deprecated property values such as ContentSwitcher's size="xl"? I think we shouldn't.

I agree. If it's no longer supported in v1, doesn't make much to sense to support it here.

Should default sizes be explicit strings such as size="md" instead of "size=undefined"? It may be better to use explicit values when writing a form generator, but they can always return size=null I suppose.

I think a prop like size should always be optional and have a consistent default value ("md" for example). We should reserve actual usage of required props where it actually makes sense (e.g., providing options for a dropdown).

metonym commented 8 months ago

After your approval, and if you prefer, I can help merge and then rebase the next/v11-styles branch, and deploy the prerelease.

I did notice discrepancies in the docs site, and I see you've listed them as known issues. I assume these will be addressed before a pre-release? I would very much like to be able to compare next with the existing master side-by-side.

theetrain commented 8 months ago

I did notice discrepancies in the docs site, and I see you've listed them as known issues.

Yeah the obvious issues are with the Search, Tile, and CodeSnippet components. Their v11 classes have been changed a bit. I believe once those components have each had a thorough look, then they will appear correctly on the docs. Another example is InlineNotification not respecting dark themes.

Would you like to deploy a separate docs site for this branch? Up to you. I'll make the first pre-release tomorrow.

metonym commented 8 months ago

@theetrain To clarify, do you mean to pre-release with the known bugs, and iterate on them in patches?

theetrain commented 8 months ago

@metonym yes, with the intent to follow up. I didn't yet go through testing every component, so it's more likely there are other issues - but that's ok. With the pre-releases, we can go through every component and make small PRs to fix what we see until eventually everything works with v11. At that point, we can merge next/v11-styles onto master.

theetrain commented 8 months ago

First pre-release has been made: v1.0.0-next-0.

See https://github.com/carbon-design-system/carbon-components-svelte/discussions/1872 for more details.

carbon-design-system / carbon-components-svelte