Fix: Fix underline issue for login and sign up buttons #190

sachinsom93 · 2021-03-07T03:37:20Z

Description

Removed the unnecessary underlines under login and sign Up buttons

Fixes #177

Type of Change:

Code

Code/Quality Assurance Only

Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Checklist:

My PR follows the style guidelines of this project
I have performed a self-review of my own code or materials

Code/Quality Assurance Only

My changes generate no new warnings

welcome · 2021-03-07T03:37:21Z

Hello there!👋 Welcome to the project!💖

Thank you and congrats🎉 for opening your first pull request.✨ AnitaB.org is an inclusive community, committed to creating a safe and positive environment.🌸Please adhere to our Code of Conduct and Contribution Guidelines.🙌.We will get back to you as soon as we can.😄

Feel free to join us on AnitaB.org Open Source Zulip Community.💖 We have different streams for each active repository for discussions.✨ Hope you have a great time there!😄

nlok5923 · 2021-03-07T16:47:17Z

src/login/Login.jsx

@@ -92,7 +92,13 @@ export default function Login() {
                        </div>
                        <div className="row button-group">
                            <div className="col">
-                                    <a className="btn btn-primary" id="registerbtn" href="/register" role="button">Sign Up</a>
+                                <Link 
+                                    to="/register"


please remove unnecessary changes.

Thanks for review this @nlok5923 , but I think it is better to use Link instead of anchor tags to navigate to some other route, what's your view @mtreacy002 ma'am, should I revert this change ?

@sachinsom93 @nlok5923 It is better to use Link in react as anchor tags trigger a page refresh which would reset the application states whereas the Link and Navlink of react-router doesn't trigger a page refresh. In single page apps with react, it is better to use Link from react-router.

@nlok5923 Just as an extra opinion, I agree with @sachinsom93 and @Rahulm2310 on the use of Link tags in place of a in React for the same reasons specified above. Generally speaking, the Link tags are better placed for in-app navigation(relative urls) and the a tags for external links(absolute urls).

nlok5923 · 2021-03-07T16:47:31Z

src/register/Register.jsx

@@ -254,7 +254,13 @@ export default function Register() {
                        </div>
                        <div className="row button-group">
                            <div className="col">
-                                    <a className="btn btn-primary" id="loginbtn" href="/login" role="button">Login</a>
+                                <Link 
+                                    to="/login"


nlok5923

please refer to the comments and also i think you should not compare package-lock.json what's your view @mtreacy002 ma'am.

Body: Text underline on the Login and Sign Up(blue) buttons(ISSUE anitab-org#177)

Rahulm2310 · 2021-03-07T18:53:43Z

@sachinsom93 Please remove package-lock.json from the commit as it has unnecessary changes which need not to be pushed.

mtreacy002 · 2021-03-13T02:46:14Z

@sachinsom93 , can you please give us an update on this?

mtreacy002

@sachinsom93 , as per @Rahulm2310 suggested, can you please remove this package-lock.json from the commit (not to delete it from your local)?

sachinsom93 · 2021-03-13T04:39:40Z

@sachinsom93 , as per @Rahulm2310 suggested, can you please remove this package-lock.json from the commit (not to delete it from your local)?

@mtreacy002 I will do it as soon as possible.

HeroicHitesh · 2021-03-18T04:06:31Z

@mtreacy002 I will do it as soon as possible.

@sachinsom93 Can you please share updates on your progress?

mtreacy002 · 2021-03-19T23:20:43Z

package.json

@@ -12,7 +12,7 @@
    "react": "^16.13.1",
    "react-dom": "^16.13.1",
    "react-router-dom": "^5.2.0",
-    "react-scripts": "^3.4.3"
+    "react-scripts": "^4.0.3"


is this update needed, @sachinsom93 ? Do you have any reason why we need this? This is also the reason why the package-lock.json is changed.

@mtreacy002 , I didn't update it manually, it got updated when I use "npm install" at the time of project setup.

@sachinsom93 We already have an issue open for updating the react-scripts version (#194). So, please revert these changes to the package.json and package-lock.json here.

This reverts commit 2d03ae5.

Rahulm2310 · 2021-03-22T17:47:03Z

@sachinsom93 , as per @Rahulm2310 suggested, can you please remove this package-lock.json from the commit (not to delete it from your local)?

@sachinsom93 you were requested to not include package-lock.json in the commit (not to delete package-lock.json). Also now I am unable to see your changes to the buttons.

The earlier changes you made to the buttons were correct. We just wanted you to not push unnecessary changes ( made to package.json and package-lock.json). If you have any doubts, then please ask.

mtreacy002 · 2021-04-10T01:27:36Z

@sachinsom93 , can you please give us an update on this?

sachinsom93 · 2021-04-15T11:08:10Z

@sachinsom93 , can you please give us an update on this?

@mtreacy002, sorry for being inactive, I tried to revert the changes but it didn't work. So, I am going to create new pr for the same.
Will it work ?

vj-codes · 2021-04-15T13:23:55Z

@sachinsom93 you can make a new PR
Also, your fork is 7 commits behind, just make sure it's in sync with develop branch before creating a new one:)

nlok5923 reviewed Mar 7, 2021

View reviewed changes

nlok5923 suggested changes Mar 7, 2021

View reviewed changes

Type: bug fix

2d03ae5

Body: Text underline on the Login and Sign Up(blue) buttons(ISSUE anitab-org#177)

mtreacy002 added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Category: User Interface Improvements or additions to design. Status: Changes Requested Changes are required to be done by the PR author. Type: Bug Bug or Bug fixes. labels Mar 8, 2021

mtreacy002 requested changes Mar 13, 2021

View reviewed changes

mtreacy002 reviewed Mar 19, 2021

View reviewed changes

sachinsom93 force-pushed the login-signup-button-issue branch from 114736e to fecfd31 Compare March 21, 2021 03:18

Revert "Type: bug fix"

fecfd31

This reverts commit 2d03ae5.

sachinsom93 closed this Apr 17, 2021

This was referenced Apr 17, 2021

Fix: text underline on the Login & Sign Up (blue) buttons #231

Merged

Bug: Test failed after Link tag is used to replace anchor tag in Register.jsx and Login.jsx #239

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Fix underline issue for login and sign up buttons #190

Fix: Fix underline issue for login and sign up buttons #190

sachinsom93 commented Mar 7, 2021

welcome bot commented Mar 7, 2021

nlok5923 Mar 7, 2021

sachinsom93 Mar 7, 2021

Rahulm2310 Mar 7, 2021

haelmj Mar 9, 2021

nlok5923 Mar 7, 2021

nlok5923 left a comment

Rahulm2310 commented Mar 7, 2021

mtreacy002 commented Mar 13, 2021

mtreacy002 left a comment

sachinsom93 commented Mar 13, 2021

HeroicHitesh commented Mar 18, 2021

mtreacy002 Mar 19, 2021 •

edited

Loading

sachinsom93 Mar 20, 2021

Rahulm2310 Mar 20, 2021

Rahulm2310 commented Mar 22, 2021 •

edited

Loading

mtreacy002 commented Apr 10, 2021

sachinsom93 commented Apr 15, 2021

vj-codes commented Apr 15, 2021

Fix: Fix underline issue for login and sign up buttons #190

Fix: Fix underline issue for login and sign up buttons #190

Conversation

sachinsom93 commented Mar 7, 2021

Description

Type of Change:

How Has This Been Tested?

Checklist:

welcome bot commented Mar 7, 2021

nlok5923 Mar 7, 2021

Choose a reason for hiding this comment

sachinsom93 Mar 7, 2021

Choose a reason for hiding this comment

Rahulm2310 Mar 7, 2021

Choose a reason for hiding this comment

haelmj Mar 9, 2021

Choose a reason for hiding this comment

nlok5923 Mar 7, 2021

Choose a reason for hiding this comment

nlok5923 left a comment

Choose a reason for hiding this comment

Rahulm2310 commented Mar 7, 2021

mtreacy002 commented Mar 13, 2021

mtreacy002 left a comment

Choose a reason for hiding this comment

sachinsom93 commented Mar 13, 2021

HeroicHitesh commented Mar 18, 2021

mtreacy002 Mar 19, 2021 • edited Loading

Choose a reason for hiding this comment

sachinsom93 Mar 20, 2021

Choose a reason for hiding this comment

Rahulm2310 Mar 20, 2021

Choose a reason for hiding this comment

Rahulm2310 commented Mar 22, 2021 • edited Loading

mtreacy002 commented Apr 10, 2021

sachinsom93 commented Apr 15, 2021

vj-codes commented Apr 15, 2021

mtreacy002 Mar 19, 2021 •

edited

Loading

Rahulm2310 commented Mar 22, 2021 •

edited

Loading