feat(#3347): add goa-scroll-panel and goa-workspace-layout#3933
feat(#3347): add goa-scroll-panel and goa-workspace-layout#3933vanessatran-ddi wants to merge 4 commits into
Conversation
a536960 to
b2f5b69
Compare
|
🎉 This PR is included in version 7.2.0-dev.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 5.2.0-dev.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
c85adcb to
5d075a5
Compare
|
Preview links
Built from commit 285b578. Previews are removed automatically when this PR closes. |
d288449 to
e4ad4d2
Compare
ac9d303 to
38c4fdb
Compare
|
Here is some feedback on the docs pages, thanks for sharing @twjeffery:
Workspace layout
|
b57fbc8 to
c9de01b
Compare
c9de01b to
aa138b4
Compare
| margin: var(--goa-modal-content-margin, 0 -2rem); | ||
| padding: var(--goa-modal-content-padding, 0); | ||
| margin: 0; | ||
| padding: 0 var(--goa-modal-scrollable-padding-desktop, var(--goa-space-l)); |
There was a problem hiding this comment.
@twjeffery and @bdfranck I also have a PR on https://github.com/GovAlta/design-tokens/pull/162/commits
Since I replace the goa-scroll to goa-scroll-panel, I need to adjust the padding og the modal action to make it better, example below: https://govalta.github.io/ui-components/pr-preview-react/pr-3933/features/3347
| overflow:hidden clips any side bleed at the edge. */ | ||
| .scroll-panel-header--shadow { | ||
| border-bottom-color: var( | ||
| --goa-scroll-panel-header-scroll-border, |
There was a problem hiding this comment.
I have this PR GovAlta/design-tokens@78c5311 to support those tokens
|
@vanessatran-ddi @twjeffery I noticed that there's no component documentation for Workspace Layout. There's an example, but nothing in components. This could be an issue:
|
|
@vanessatran-ddi @twjeffery There's no image for the Scroll Panel in the All Components page. |
| // Hooks | ||
| // ***** | ||
| onMount(() => { | ||
| if (!isValidDimension(height)) { |
There was a problem hiding this comment.
There's a potential problem here, in the docs it says height can be "Any valid CSS height value (e.g. "400px", "100%", "100vh")". This statement is incorrect. The function isValidDimension fails to account for calc(), dvh, lvh, or svh, min(), or clamp(). If I use a technical valid value out of that list, it does actually still work, but states a console error incorrectly. I don't think the option is to allow those values in isValidDimension, because that would require far more testing wherever that function is used, but maybe we be more clear about what values are allowed in the documentation.
In addition, if I use an actual invalid value for height, it still proceeds to try to create the ScrollPanel, the check doesn't actually stop anything. This results in an area in the screen that you can't actually scroll down or up in, it prevents page scrolling, and takes up the full height of the content.
There was a problem hiding this comment.
I pushed a test at the bottom of the React feat3347.tsx that shows the second problem.
4dea910 to
17bc7f1
Compare
| alt="Complete workspace layout preview" | ||
| previewUrl="/examples/workspace-layout/basic/preview" | ||
| figmaUrl="https://www.figma.com/design/Jpy1Ea5qglwnp1SgGnagY9/%E2%9D%96-Component-library?node-id=58855-102956" | ||
| reactSourceUrl="https://github.com/GovAlta/ui-components/blob/vanessa/3347-sticky-header-footer/docs/src/content/examples/workspace-layout/basic/react.tsx" |
There was a problem hiding this comment.
This URL and the URL below it will cease to exist once this PR is merged in (because that deletes the PR).
bdfranck
left a comment
There was a problem hiding this comment.
I tested the latest changes...
| Browser | Drawer | Modal | Notification |
|---|---|---|---|
| Chrome | ✅ | ✅ | ✅ |
| Safari | ✅ | ✅ | ✅ |
| Firefox | ✅ | ✅ | ✅ |
| Safari mobile | ✅ | ❓ | ✅ |
@vanessatran-ddi It looks like the padding is missing for a Modal on mobile. Or maybe I have the wrong tokens?


Before (the change)
After (the change)
Make sure that you've checked the boxes below before you submit the PR
Steps needed to test
https://govalta.github.io/ui-components/pr-preview/pr-3933/examples/workspace-layout/
https://govalta.github.io/ui-components/pr-preview/pr-3933/components/scroll-panel/#react#properties
React Links:
https://govalta.github.io/ui-components/pr-preview-react/pr-3933/features/3347
https://govalta.github.io/ui-components/pr-preview-react/pr-3933/features/3347-push
Angular Links:
https://govalta.github.io/ui-components/pr-preview-angular/pr-3933/features/3347-push
https://govalta.github.io/ui-components/pr-preview-angular/pr-3933/features/3347