Conversation
rammodhvadia
temporarily deployed
to
previews/1402/merge
GitHub Actions
Inactive
rammodhvadia
temporarily deployed
to
previews/1402/merge
GitHub Actions
Inactive
rammodhvadia
temporarily deployed
to
previews/1402/merge
GitHub Actions
Inactive
rammodhvadia
temporarily deployed
to
previews/1402/merge
GitHub Actions
Inactive
rammodhvadia
temporarily deployed
to
previews/1402/merge
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
Enables injecting a “final step” view into the Projects instructions panel by adding new styling/assets and adjusting the build to support the required SVG.
Changes:
- Added
EditorFinalStep.scssfor the new instruction step UI styling. - Extended instructions styling to support primary design-system button presentation.
- Added
cc-wallpaper.svgand increased webpack SVG inline limit to accommodate it.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.config.js | Increases SVG url-loader inline limit to support the new wallpaper asset. |
| src/components/Menus/Sidebar/InstructionsPanel/InstructionsPanel.jsx | Minor formatting-only change. |
| src/assets/stylesheets/InternalStyles.scss | Includes the new final-step stylesheet and reformats CSS variable declarations. |
| src/assets/stylesheets/Instructions.scss | Adds .rpf-button styling within .project-instructions for primary button appearance. |
| src/assets/stylesheets/EditorFinalStep.scss | Introduces styles for the new final step “what’s next” content/card. |
| src/assets/cc-wallpaper.svg | Adds the wallpaper SVG referenced by the new final-step styling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| loader: "url-loader", | ||
| options: { | ||
| limit: 10000, | ||
| limit: 100000, |
There was a problem hiding this comment.
Raising the global .svg url-loader inline limit to 100000 will cause many more SVGs to be embedded into JS/CSS as data URIs, which can noticeably increase bundle size and memory usage. Consider scoping the higher limit to just the one asset that needs it (or letting larger SVGs emit as separate files) to avoid inlining unrelated SVGs.
| limit: 100000, | |
| limit: 10000, |
There was a problem hiding this comment.
What do you think about these suggestion @rammodhvadia ?
Inlining larger assets to the bundle adds to the weight make and give browsers more work to parse. Yes the limit is arbitrary, but increasing it makes it more likely we will add larger assets in the future too.
It would be valuable to be able to refer to SVGs using a URL rather than embedding them.
| align-items: center; | ||
| background-color: var(--cc-light-cyan); | ||
| background-image: url("../cc-wallpaper.svg"); | ||
| background-position: center; | ||
| background-size: cover; | ||
| border-radius: var(--space-3); | ||
| display: flex; | ||
| flex-direction: column; | ||
| gap: var(--space-3); | ||
| overflow: hidden; | ||
| padding-block: var(--space-5); | ||
| padding-inline: var(--space-4); | ||
| position: relative; | ||
|
|
||
| &::before { | ||
| background: linear-gradient( | ||
| 90deg, | ||
| transparent 0%, | ||
| color-mix(in srgb, var(--cc-light-cyan) 90%, transparent) 20%, | ||
| color-mix(in srgb, var(--cc-light-cyan) 100%, transparent) 50%, | ||
| color-mix(in srgb, var(--cc-light-cyan) 90%, transparent) 80%, | ||
| transparent 100% | ||
| ); |
There was a problem hiding this comment.
--cc-light-cyan is not defined anywhere in this repo, so background-color: var(--cc-light-cyan) (and the color-mix() calls that depend on it) will compute to invalid/empty values unless the host app provides the variable. Add a fallback value (or use an existing design-system color token), and consider providing a non-color-mix() gradient fallback because color-mix() is not supported in all browsers covered by the current browserslist.
rammodhvadia
temporarily deployed
to
previews/1402/merge
GitHub Actions
Inactive
zetter-rpf
left a comment
zetter-rpf
left a comment
There was a problem hiding this comment.
Thanks investigating the alternative ways to include the asset
A few changes to enable us to inject a final step to the instructions panel from projects-ui:
EditorFinalStep.scsswhich has all the styling this new instruction step might needInstructions.scsscc-wallpaper.svgneeded for one of the components being injected in the new final stepQuestions/suggestions welcome :)