🔧 .container.is-fluid to tailwind #11391
Conversation
roiLeo
requested review from
vikiival and
Jarsen136
and removed request for
a team
✅ Deploy Preview for koda-canary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for polkadot ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
vikiival
requested review from
preschian and
hassnian
and removed request for
vikiival
There was a problem hiding this comment.
@roiLeo I think we should add container-fluid as a utility class
https://v3.tailwindcss.com/docs/adding-custom-styles#adding-custom-utilities
It's much cleaner and more maintainable, wdyt?
IMO, best solution would be to create a container wrapper component but that not the point of this PR. |
i'm not opposed to that, feel free to raise a pr. |
WalkthroughThis pull request unifies the layout styling across various components and layouts by replacing the Changes
Sequence Diagram(s)sequenceDiagram
participant GIL as GalleryItemLayout
participant UD as useDevice Composable
GIL->>UD: Request device type (isMobileOrTablet)
UD-->>GIL: Return isTouch status
GIL->>GIL: Apply conditional classes based on isTouch
GIL->>Template: Render updated layout
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
layouts/gallery-item-layout.vue (1)
6-7: Consider extracting container styles into a reusable component.The class combinations are duplicated in
Navbar.vue. Consider creating a wrapper component to promote reusability.libs/ui/src/scss/tailwind.scss (1)
151-153: Attention: Validate!importantUsage Syntax in.container-fluidUtilityThe new
.container-fluidutility class correctly aggregates the desired Tailwind CSS utilities. However, the usage of#{!important}in line 152 to enforce the!importantflag is nonstandard and may lead to SCSS compilation issues. Typically, if you want to enforce!important, you would append it directly (e.g.,px-5 !important) or handle this via Tailwind’s configuration. Please verify that your build system processes this interpolation as intended, and consider revising it if any issues arise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
assets/styles/global.scss(1 hunks)components/ExploreLayoutWithSidebar.vue(1 hunks)components/Navbar.vue(1 hunks)components/collection/CollectionHeader.vue(1 hunks)components/collection/CollectionHeader/CollectionBanner.vue(1 hunks)components/landing/HeroBanner.vue(2 hunks)components/landing/LandingPage.vue(1 hunks)components/swap/layout/index.vue(1 hunks)components/swap/review.vue(1 hunks)layouts/default.vue(1 hunks)layouts/explore-layout.vue(1 hunks)layouts/gallery-item-layout.vue(2 hunks)layouts/no-footer.vue(1 hunks)layouts/teleport-layout.vue(1 hunks)libs/ui/src/scss/tailwind.scss(1 hunks)pages/[prefix]/explore/collectibles.vue(1 hunks)
✅ Files skipped from review due to trivial changes (11)
- components/swap/review.vue
- components/swap/layout/index.vue
- components/collection/CollectionHeader.vue
- components/collection/CollectionHeader/CollectionBanner.vue
- layouts/default.vue
- components/ExploreLayoutWithSidebar.vue
- pages/[prefix]/explore/collectibles.vue
- components/landing/HeroBanner.vue
- layouts/no-footer.vue
- layouts/teleport-layout.vue
- components/landing/LandingPage.vue
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Redirect rules
- GitHub Check: Header rules
- GitHub Check: Pages changed
- GitHub Check: test
- GitHub Check: build
- GitHub Check: build-cf
- GitHub Check: shard (1, 10)
- GitHub Check: runner / eslint
🔇 Additional comments (8)
layouts/gallery-item-layout.vue (1)
39-39: LGTM! Good use of device detection.The change from
useWindowSizetouseDeviceis a better approach for responsive design.layouts/explore-layout.vue (2)
16-16: LGTM! Consistent with container class changes.The change from
container is-fluidtocontainer-fluidaligns with the PR objectives.
16-16: LGTM!The class name change from
container is-fluidtocontainer-fluidaligns with the PR's objective to migrate to Tailwind CSS.components/Navbar.vue (4)
11-12: Consider extracting container styles into a reusable component.The class combinations are duplicated in
gallery-item-layout.vue. Consider creating a wrapper component to promote reusability.
15-15: LGTM! Good use of negative margin.The negative margin adjustment improves the brand logo positioning.
11-12: LGTM! Improved responsive layout using Tailwind utilities.The changes enhance layout control and responsiveness by:
- Using flex utilities for better spacing and alignment
- Adding responsive padding adjustments
- Implementing touch-specific conditional classes
15-15:❓ Verification inconclusive
Verify the negative margin adjustment.
The negative margin
ml-[-0.75rem]on the navbar brand might affect the layout on different screen sizes.Please verify that:
- The logo remains properly aligned across all breakpoints
- There's no content overflow or clipping
- The spacing remains consistent with the design system
🏁 Script executed:
#!/bin/bash # Description: Search for other instances of negative margins in the codebase to ensure consistency # Search for negative margin patterns in style files rg -A 2 'ml-\[-.*\]|margin-left: -' --type css --type scss --type vueLength of output: 100
Action Required: Re-run the Verification Script with Updated File Type Handling
The original script did not account for SCSS files properly (resulting in the “unrecognized file type: scss” error). Please re-run the following updated script, which adds SCSS support explicitly, to verify that the negative margin usage is consistent across the codebase:
Once this script executes successfully, please manually verify that:
- The logo remains correctly aligned at all breakpoints.
- No content overflow or clipping occurs.
- The spacing stays consistent with the design system.
🏁 Script executed:
[run_scripts] #!/bin/bash # Re-run the search for negative margins with correct SCSS support rg -A 2 'ml-\[-.*\]|margin-left: -' --type-add "scss:*.scss" --type scss --type css --type vueLength of output: 246
Re-run Verification for Negative Margin on Navbar Brand
The current script encountered an error due to the unrecognized vue file type. Please re-run the following updated script using file globs to ensure that all relevant files (Vue, SCSS, CSS) are checked for negative margin usage:
Once executed, please manually verify that:
- The logo remains properly aligned across all breakpoints.
- There is no content overflow or clipping.
- The spacing is consistent with the design system.
🏁 Script executed:
[run_scripts] #!/bin/bash # Re-run search for negative margins across Vue, SCSS, and CSS files using file globs rg -A 2 'ml-\[-.*\]|margin-left: -' -g '*.vue' -g '*.scss' -g '*.css'Length of output: 376
Double-check Negative Margin Impact on Navbar Layout
The negative margin
ml-[-0.75rem]incomponents/Navbar.vue(line 15) is the only occurrence in the codebase. Please verify that:
- The logo remains properly aligned across all responsive breakpoints.
- There is no content overflow or clipping.
- The spacing is consistent with the design system.
Manually confirm the layout behavior on different screen sizes to ensure no unexpected issues arise.
assets/styles/global.scss (1)
203-203: LGTM! Clean removal of legacy container styles.The removal of
.container.is-fluidand mobile-specific styles aligns with the transition to Tailwind utility classes.
part of:
Summary by CodeRabbit