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.

Sign up for GitHub

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

Jump to bottom

Sign in & Sign Up buttons enhancement #168

Merged
merged 4 commits into from
Feb 17, 2023
Merged

Sign in & Sign Up buttons enhancement #168

merged 4 commits into from
Feb 17, 2023

Conversation

ritika728
Copy link
Contributor

Hey, I have resolved issue #166. Thank you!

github-actions[bot]
github-actions bot reviewed Feb 16, 2023
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hi there, Thanks a lot for your first pull request, we'll be reviewing it asap :)

@narayan954 narayan954 linked an issue Feb 16, 2023 that may be closed by this pull request
Enhancing the sign up and sign in button #166
Closed
narayan954
narayan954 requested changes Feb 16, 2023
View reviewed changes
Copy link
Owner

@narayan954 narayan954 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Using reserved prop-names like className and style on custom components hurts readability and makes the code harder to maintain. These props should only be used for DOM nodes like div, a, span, etc. It works in this case but should not be used in practice @ritika728 .
You can take a look into this https://mui.com/material-ui/customization/how-to-customize/

@ritika728
Copy link
Contributor Author

Thank you for the information! I have made the required changes. Kindly review and let me know if any more changes are needed @narayan954

@narayan954
Copy link
Owner

narayan954 commented Feb 16, 2023 •
edited
Loading

Thank you for the information! I have made the required changes. Kindly review and let me know if any more changes are needed @narayan954

You did great :) but you forgot the DRY principal! You can store that style in a constant variable.

@narayan954 narayan954 self-requested a review February 17, 2023 13:12
@narayan954
encapsulated button style
e89af2c 
stored style for button in a variable to remove repetition of code
narayan954
narayan954 approved these changes Feb 17, 2023
View reviewed changes
Copy link
Owner

@narayan954 narayan954 left a comment

Choose a reason for hiding this comment

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

you may take a look at this.

@narayan954 narayan954 merged commit e615d24 into narayan954:master Feb 17, 2023
@ritika728
Copy link
Contributor Author

you may take a look at this.

Yes, I got it, thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions left review comments

@narayan954 narayan954 narayan954 approved these changes

Assignees
No one assigned
Labels
easy
Projects
Dummygram
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enhancing the sign up and sign in button
2 participants
@ritika728 @narayan954