Skip to content
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

An initial Version of my Component Library #1

Open
wants to merge 45 commits into
base: main
Choose a base branch
from
Open

Conversation

hrshmistry
Copy link
Owner

An initial version of my Component Library,

Hello everyone I have done my initial version of Component Library, I tried to make it responsive to mobile yet things like the menu are yet to be added. I tried to follow best practices but I need more practice so your feedback is important!

please give your reviews and feedback, where can I improve it.

Deployed link of development branch: Compact CSS

Thanks.

@hrshmistry hrshmistry changed the title Development Development: An initial Version of my Component Library Mar 30, 2021
@hrshmistry hrshmistry changed the title Development: An initial Version of my Component Library An initial Version of my Component Library Mar 30, 2021
@shubambhasin
Copy link

I have reviewed your cmponent lib, everything is nicely done 👌

  • just a small suggestion in the lib wesbite, the side nav goes a little up while scrolling, you can fix it. Rest is just fine

Copy link

@tuhindas30 tuhindas30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @hrshmistry,
Such a cool library you made. Loved it. The avatar and card section was quite attractive to the eye. Bohot e acchaa.

Few comments after going through:

  • It is always a good idea to remove all commented out code. I have commented in one of the files but you should do it for all the files.
  • rem makes thing responsive a bit. So, it is better to use that. px can be used when no other options are available.

Again, nicely organised code and good naming conventions. Nothing very serious. Just some small stuffs :)

Comment on lines +6 to +7
/* align-items: center;
height: 700px; */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's always a good practice to remove commented-out code to avoid distraction to the reviewer.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes absolutely, I will remove the commented code and refactor it.

Comment on lines +4 to +7
border-radius: 50%;
width: 50px;
height: 50px;
margin: 0 0.5rem;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to follow a particular sizing unit inside a class or inside an attribute. rem makes things more responsive whereas px doesn't.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing it makes sense to use rem most of the time. I will change accordingly.

@hrshmistry
Copy link
Owner Author

hrshmistry commented Mar 31, 2021

I have reviewed your cmponent lib, everything is nicely done 👌

  • just a small suggestion in the lib wesbite, the side nav goes a little up while scrolling, you can fix it. Rest is just fine

@shubambhasin Glad, you liked it, I have also noticed that issue Now fixing it soon and yes thank you for reviewing 😄

@hrshmistry
Copy link
Owner Author

Hi @hrshmistry,
Such a cool library you made. Loved it. The avatar and card section was quite attractive to the eye. Bohot e acchaa.

Few comments after going through:

  • It is always a good idea to remove all commented out code. I have commented in one of the files but you should do it for all the files.
  • rem makes thing responsive a bit. So, it is better to use that. px can be used when no other options are available.

Again, nicely organised code and good naming conventions. Nothing very serious. Just some small stuffs :)

Thank you @tuhindas30 😁, currently there are only a few components in the library but I will implement more thanks again for reviewing 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants