-
Notifications
You must be signed in to change notification settings - Fork 25
refactor: gaps to tailwind #1136
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
Conversation
b6cbf49 to
deae168
Compare
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.
Pull Request Overview
This PR refactors gap utilities throughout the codebase by replacing custom CSS gap values with standardized Tailwind CSS gap utilities, as part of issue #937. The changes improve consistency and maintainability by removing custom SCSS gap definitions in favor of Tailwind's standardized spacing system.
- Migrated button components from custom
oc-button-gap-*classes to Tailwindgap-*utilities - Replaced CSS
gapdeclarations in SCSS with corresponding Tailwind gap classes - Updated test snapshots to reflect the new gap class names throughout component renders
Reviewed Changes
Copilot reviewed 70 out of 70 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web-runtime/src/components/Topbar/*.vue | Migrated topbar components from custom gap styling to Tailwind gap utilities |
| packages/web-pkg/src/components/**/*.vue | Updated various components to use Tailwind gap classes instead of custom CSS |
| packages/web-app-files/src/components/**/*.vue | Refactored file management components to standardized gap utilities |
| packages/web-app-/src/**/.vue | Updated app-specific components to use Tailwind gap system |
| tests/**/snapshots/*.snap | Updated test snapshots to reflect new gap class names |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
packages/web-pkg/tests/unit/components/Filters/__snapshots__/DateFilter.spec.ts.snap
Outdated
Show resolved
Hide resolved
packages/web-pkg/tests/unit/components/Filters/__snapshots__/DateFilter.spec.ts.snap
Show resolved
Hide resolved
575b7ec to
51764e6
Compare
| &-item { | ||
| gap: 8px; | ||
| } |
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.
Removed, is already set on a button.
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.
we have getTailwindSizeClass in design-system/src/helpers/sizeClasses.ts. Can't we place the gap helper there as well instead of a new file? Or move the getTailwindSizeClass helper into this new tailwind.ts file. ;-)
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.
I have a PR coming up that removes the getTailwindSizeClass helper. We must not use interpolation with Tailwind classes because Tailwind doesn't recognize these classes and they might be missing in the bundle therefore.
| </div> | ||
| </div> | ||
| <oc-list class="oc-tiles grid justify-start"> | ||
| <oc-list class="oc-tiles grid justify-start gap-3"> |
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.
you got me confused: here you have gap-3 for 1rem, in other places you had gap-2 for 1rem. What's correct now?
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.
gap-3 is the 1:1 adaption for 1rem. However, I found gap-2 fitting for the other place (I think it was the tile grid in the app store).
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.
Okay I checked again. Removed one of them (didn't do anything because elements are not side by side) and set the other to gap-3 as well.
kulmann
left a comment
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.
🥳
refactor: gaps to tailwind
refs #937