-
Notifications
You must be signed in to change notification settings - Fork 101
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
feat: Implemented border when hover #557
Conversation
✅ Deploy Preview for java-processing-faf822 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Added spacing between each content items in the listUpdated Screenshot for #508 20240826180247.mp4 |
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.
Hi Rishabdev,
Thank you for your contribution, your changes are in the right direction but need some adjustments
package.json
Outdated
@@ -71,6 +71,7 @@ | |||
"react-dom": "^17.0.2", | |||
"react-helmet": "^6.1.0", | |||
"react-intl": "^5.25.1", | |||
"react-refresh": "^0.14.2", |
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.
Not part of this pr
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.
When I ran the application it showed this -
**ERROR in polyfill
Module not found: Error: Cannot find module 'react-refresh'
that's why i installed that dependency.
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.
Can you suggest what can be done ?
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 can't replicate that issue, what's your browser/system/node version etc...?
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.
browser - chrome
system - windows 11
node - v19.6.1
Also @SableRaf lmk if the visual changes are approved, especially regarding the spacing and fontsize changes |
Thanks @Stefterv! Yes it looks good to me visually. |
src/components/Sidebar.module.css
Outdated
&::before { | ||
content: ''; | ||
position: absolute; | ||
left: 0; | ||
top: 50%; | ||
height: calc(100% - 20%); | ||
width: 4px; | ||
background-color: transparent; | ||
transition: background-color 0.3s; | ||
transform: translateY(-50%); |
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.
This entire section is not used
Hey @rishabdev2997! Just checking in—are you still interested in finishing up this PR? Let me know; otherwise, I'll go ahead and close it. Thanks! |
@SableRaf I will finish it in 3 days. sorry for late reply |
Sure thing! No worries. Thanks for your reply :) let us know if you need any support. |
I will make a new pr with the updates |
feat: Highlight or add left border to active navbar subitem
#487 - added the feature.
In the issue the problem was not mentioned properly(the download nav item gets a left border for distinction. The same can apply to nav sub-items.) but it seems it indicates navs under the "Table of Contents" section must display the same left border upon selection.
How to see the changes
Here is the Screenshot
20240826172905.mp4