-
Notifications
You must be signed in to change notification settings - Fork 6
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
made ui changes #28
made ui changes #28
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe project has undergone a visual refresh, with updates to the styling of components and the overall background. The Changes
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 X ? TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- package-lock.json
Files selected for processing (4)
- components/style/index.js (1 hunks)
- pages/index.js (3 hunks)
- styles/Home.module.css (5 hunks)
- styles/globals.css (1 hunks)
Files skipped from review due to trivial changes (1)
- styles/globals.css
Additional comments: 5
components/style/index.js (1)
- 15-20: The updated styles for
StyledCard
with the newbackground
,boxShadow
, andmargin
properties seem consistent with the PR's objective to update the UI. Ensure that the new margin does not cause layout issues in the context whereStyledCard
is used.pages/index.js (2)
5-5: The addition of the
Roboto
font import aligns with the UI changes. Verify that the font is being used correctly across the application.154-164: The addition of a
button
element with classstyles.button
and aspan
element with classstyles.buttontop
should enhance the navbar's UI. Verify that these changes are visually and functionally integrated well with the existing navbar design.styles/Home.module.css (2)
57-84: The new styles for
.button
,.buttontop
, and the hover effect for.btn:hover
are in line with the PR's UI update objectives. Ensure that the hover effect transitions smoothly and the button's appearance is consistent across different browsers and devices.197-223: The modifications to the
.card
and.card2
classes, including the hover effects, seem to be part of the intended UI enhancements. Verify that these changes do not conflict with other styles and that the hover effects are accessible and performant.
This PR is missing a Jira ticket reference in the title or description. |
Please remove the |
removed the packagelock.json file from commits |
Please have a look on the review comments too. |
please explain can't understand ...a lot said by the bot |
Remove extra spaces and blank line. |
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (5)
- components/style/index.js (3 hunks)
- pages/footer.js (1 hunks)
- pages/index.js (5 hunks)
- styles/Home.module.css (6 hunks)
- styles/globals.css (2 hunks)
Files skipped from review due to trivial changes (2)
- pages/footer.js
- styles/globals.css
Files skipped from review as they are similar to previous changes (2)
- components/style/index.js
- pages/index.js
Additional comments: 4
styles/Home.module.css (4)
53-81: The new styles for
.button
and.buttontop
introduce variables for radius and color, which is good for maintainability. However, the hover effect on.button:hover .buttontop
changes thetransform
property, which could lead to layout shifts. Verify that this hover effect does not cause any unwanted movement of surrounding elements on the page.130-140: The hover effect on
.btn:hover
uses a significant translateY value and multiple box shadows, which could be performance-intensive on paint and layout. Consider verifying the performance impact on low-powered devices and possibly simplifying the effect if it causes slowdowns.191-210: The new styles for
.card
and.card2
include hover effects that change thetransform
andbox-shadow
properties. Ensure that these effects are smooth and do not cause layout shifts. Additionally, thebox-shadow
on.card:hover
might be performance-intensive due to the large blur radius. It's important to test these effects on various devices to ensure they do not negatively impact performance.395-400: The media query for a maximum width of 700px adjusts the text alignment and margin for
.card h2
. Ensure that these changes do not cause any text overflow or alignment issues on smaller screens.
done sir |
Nice work, just want to know one thing. Can't we remove the overflow? |
Yes sir we can , but can you please be specific what kind of overflow have you encountered |
@GAGANRAGHAV Overflow in y-axis |
sir are you saying to make the full webpage visible without scrol ? |
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.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (1)
- styles/Home.module.css (7 hunks)
Additional comments: 3
styles/Home.module.css (3)
66-75: The
.buttontop
class is introduced for styling a specific part of the button. Thetransform
property is used for a subtle effect. Ensure that this effect is consistent with the UX design guidelines of the project. Also, thetransition
property is set to affect only thetransform
property, which is a good practice for performance.78-80: The hover effect for
.button:hover .buttontop
is specified to move the button upwards. This is a good use of thetransform
property for a performant hover effect. However, ensure that the hover effect's magnitude (-0.33em
) is consistent with the design specifications.191-195: The
.card
class has been updated with new dimensions and a gradient background. Ensure that these dimensions do not conflict with the layout on different screen sizes and that the gradient is accessible in terms of color contrast.
.button{ | ||
--button_radius: 0.75em; | ||
--button_color: #e8e8e8; | ||
--button_outline_color: #000000; | ||
font-size: 17px; | ||
font-weight: bold; | ||
border: none; | ||
border-radius: var(--button_radius); | ||
background: var(--button_outline_color); | ||
cursor: pointer; | ||
} |
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.
The new .button
class includes a custom property --button_radius
which is good for maintainability, but there's a potential issue with the --button_color
and --button_outline_color
being defined within the class. These should be defined at the root level to ensure consistency across the application and ease of theming.
- --button_radius: 0.75em;
- --button_color: #e8e8e8;
- --button_outline_color: #000000;
+ :root {
+ --button_radius: 0.75em;
+ --button_color: #e8e8e8;
+ --button_outline_color: #000000;
+ }
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
.button{ | |
--button_radius: 0.75em; | |
--button_color: #e8e8e8; | |
--button_outline_color: #000000; | |
font-size: 17px; | |
font-weight: bold; | |
border: none; | |
border-radius: var(--button_radius); | |
background: var(--button_outline_color); | |
cursor: pointer; | |
} | |
:root { | |
--button_radius: 0.75em; | |
--button_color: #e8e8e8; | |
--button_outline_color: #000000; | |
} | |
.button{ | |
font-size: 17px; | |
font-weight: bold; | |
border: none; | |
border-radius: var(--button_radius); | |
background: var(--button_outline_color); | |
cursor: pointer; | |
} |
.btn:hover{ | ||
border-radius: 5px; | ||
transform: translateY(-10px); | ||
box-shadow: 0 7px 0 -2px #f85959, | ||
0 15px 0 -4px #39a2db, | ||
0 16px 10px -3px #39a2db; |
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.
The .btn:hover
class has multiple box-shadow effects which could be a performance concern on low-powered devices or with many elements on the page. Consider simplifying the shadows or using a will-change optimization if this proves to be a performance bottleneck.
+ will-change: transform, box-shadow;
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
.btn:hover{ | |
border-radius: 5px; | |
transform: translateY(-10px); | |
box-shadow: 0 7px 0 -2px #f85959, | |
0 15px 0 -4px #39a2db, | |
0 16px 10px -3px #39a2db; | |
.btn:hover{ | |
border-radius: 5px; | |
transform: translateY(-10px); | |
will-change: transform, box-shadow; | |
box-shadow: 0 7px 0 -2px #f85959, | |
0 15px 0 -4px #39a2db, | |
0 16px 10px -3px #39a2db; |
width: 200px; | ||
height: 150px; | ||
background-color: #1a1a1a; | ||
padding: 1rem 1.2rem; | ||
|
||
.card span { | ||
} | ||
.card2:hover { | ||
transform: scale(0.98); | ||
border-radius: 20px; | ||
} |
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.
The .card2
class and its hover effect have been added. The transform: scale(0.98);
on hover might not be noticeable to the user. Consider if a more significant scale change or different effect would better suit the intended user experience.
background-image: linear-gradient(163deg, #00ff75 0%, #3700ff 100%); | ||
box-shadow: 0px 0px 30px 1px rgba(0, 255, 117, 0.30); |
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.
The hover effect for .card:hover
is redefining the background image which is already defined in the .card
class. This is redundant and could be removed to clean up the code.
- .card:hover{
- background-image: linear-gradient(163deg, #00ff75 0%, #3700ff 100%);
- }
Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
background-image: linear-gradient(163deg, #00ff75 0%, #3700ff 100%); | |
box-shadow: 0px 0px 30px 1px rgba(0, 255, 117, 0.30); | |
`` | |
</details> | |
<!-- suggestion_end --> | |
<!-- This is an auto-generated comment by CodeRabbit --> |
sir i have made some changes please check. |
Nice work. Still need improvements in UI/UX but it was good work. Thanks for contributing. |
Reverting this PR @GAGANRAGHAV as UI/UX is not looking good in mac. Please work on it. |
Sir please suggest some as i don't have mac . |
Description
made ui changes.
Fixes # 27
Type of change
ui/ux
screenshot---
before--
after--
https://www.loom.com/share/bb290e57bb5042ea9b9d5a07a84fd44b
Checklist:
Summary by CodeRabbit
StyledCard
component with new background color, shadow, and margin adjustments for a refreshed look.StyledButton
on hover for a cleaner design.Roboto
font for improved text readability across various components.Home
component with additional button elements and refined styling for better user interaction.