-
Notifications
You must be signed in to change notification settings - Fork 267
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
Update to Carbon 11 #3501
Update to Carbon 11 #3501
Conversation
Skipping CI for Draft Pull Request. |
Also remove `@carbon/themes` as we'll consume that via `@carbon/react` instead
Also replace custom `getCarbonPrefix` util with the provided context and hook.
Note: skipping step 2 as we're already using dart sass - Update the base imports to use the new package. - Minimal changes to resolve warnings about unknown imports (mostly the component styles) - Fix the theme classes
At this point the application builds and loads successfully in the browser. The styling is far from perfect but the major pieces are in place. There are also a number of functional issues due to changes in component APIs / behaviours, e.g. tab lists do not currently render.
…interactive content
This concludes the short version of the migration guide, for the core application only (graph package will be done later). Unit test status: ``` Test Files 20 failed | 53 passed (73) Tests 57 failed | 524 passed | 1 skipped (582) ```
The `$fallback` config is meant for use with the compatibility theme.
Temporarily disable the graph stories. That package will be upgraded later.
These may be update to the new CSS grid-based Grid component later.
Update the ResourceDetails, StepDetails, and TaskRun details stories to use the Storybook API to update the `view` prop when selecting a tab.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the walkthrough. A few queries, but looking good otherwise
packages/components/src/components/PipelineRuns/PipelineRuns.stories.jsx
Show resolved
Hide resolved
This wasn't handled correctly by the Carbon code mods during migration. Manually restore the expected size.
/test all |
When navigating the tab list with the arrow keys, the default behaviour of the Carbon components is to load the tab panel corresponding with the currently focused tab. Since some of the tab content in the Dashboard triggers additional network requests when rendered (e.g. step logs tab), set `activation="manual"` so that the user must click (or press space / enter) to activate the tab.
Includes fixes for: - Unexpected page scroll when using arrow keys to navigate MenuButton items - Wrong tab styles in Firefox
/hold cancel |
/test all |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
👍
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: briangleeson The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Changes
Closes #3493
/kind misc
Some of the remaining todo items are captured below.
There are more annotatedTODO: carbon11
directly in the code.Carbon:
<Stack>
for vertical spacing@use '@carbon/react/scss/fonts';
@font-face
s)Dashboard:
tkn--visually-hidden
withcds--visually-hidden
Tests:
with graph components excludedmoststories functionalsome stories not rendering (e.g. resourcedetails default)some functional issues (unable to switch tabs in some stories)Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
/kind <type>
. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes