fix: fixed the gap issue in 2nd onboarding screen ui#3300
fix: fixed the gap issue in 2nd onboarding screen ui#3300Dhruwang merged 9 commits intoformbricks:mainfrom
Conversation
|
@abhayofc is attempting to deploy a commit to the formbricks Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe changes in the pull request focus on the Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
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 (
|
github-actions
bot
added
bug
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apps/web/app/(app)/(onboarding)/organizations/components/OnboardingOptionsContainer.tsx (2)
45-47: Approved: Layout change addresses the gap issue effectively.The transition from a grid to a flexbox layout effectively addresses the PR objective of fixing the gap issue in the 2nd onboarding screen UI. The conditional class application ensures appropriate layout for different numbers of options, and the use of
justify-centershould help in reducing gaps between boxes when zoomed out.Consider adding a
flex-wrapclass to improve responsiveness on smaller screens:- "flex w-5/6 justify-center gap-8 text-center md:flex-row lg:w-2/3": options.length >= 3, - "flex justify-center gap-8": options.length < 3, + "flex flex-wrap w-5/6 justify-center gap-8 text-center md:flex-row lg:w-2/3": options.length >= 3, + "flex flex-wrap justify-center gap-8": options.length < 3,This change will allow the options to wrap to the next line on smaller screens, preventing horizontal scrolling.
Line range hint
27-27: Approved: Improved prop handling forOptionCard.The explicit setting of the
loadingprop tooption.isLoading || falseis a good practice. It ensures that theloadingprop defaults tofalseifisLoadingis not provided, improving the component's robustness.Consider using TypeScript's non-null assertion operator to make the code more concise while maintaining type safety:
- loading={option.isLoading || false} + loading={option.isLoading!}This change assumes that
isLoadingis always defined in theoptionobject, which should be enforced in theOnboardingOptionsContainerPropsinterface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
- apps/web/app/(app)/(onboarding)/organizations/components/OnboardingOptionsContainer.tsx (1 hunks)
🔇 Additional comments (1)
apps/web/app/(app)/(onboarding)/organizations/components/OnboardingOptionsContainer.tsx (1)
Line range hint
1-58: Summary: PR successfully addresses the UI layout issue.Overall, the changes effectively solve the gap issue in the 2nd onboarding screen UI as intended. The transition from a grid to a flexbox layout, along with the use of
justify-center, should reduce gaps between boxes when zoomed out. The code maintains good practices and even improves robustness with the explicit handling of theloadingprop.Minor suggestions for improvements have been made, including:
- Adding
flex-wrapfor better responsiveness on smaller screens.- Using TypeScript's non-null assertion operator for more concise prop handling.
These changes successfully meet the PR objectives while maintaining code quality and responsiveness.
|
I didn't know that the pnpm.lock file will be ignored so I opened another commit for deleting that file, does this affect something? |
jobenjada
left a comment
jobenjada
left a comment
There was a problem hiding this comment.
love the UI change!
but we need our pnpm lock file 😬
please update your PR to contain only the chance we want
|
im just converting it to draft to keep an overview |
|
@jobenjada I have added the pnpm-lock file and also fixed #3304 |
|
Awarding abhayofc: 150 points 🕹️ Well done! Check out your new contribution on oss.gg/abhayofc |
3 participants
What does this PR do?
This PR fixes the UI of the second onboarding page UI while zooming it out. It reduces the gaps between the boxes.
Fixes #3299
Formbricks.-.Google.Chrome.2024-10-06.00-25-20.webm
How should this be tested?
You can test this fix by following these steps:
Checklist
Required
pnpm buildconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Bug Fixes
falseif not specified, improving reliability during loading scenarios.Enhancements