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

fix(UI): Button alignment in email signup #52132

Merged
merged 5 commits into from Nov 6, 2023

Conversation

JeevaRamanathan
Copy link
Contributor

@JeevaRamanathan JeevaRamanathan commented Oct 27, 2023

Closes #52131

The change involves fixing the misalignment of the button in the email sign up page.
Screenshots:
image

@JeevaRamanathan JeevaRamanathan requested a review from a team as a code owner October 27, 2023 07:18
@github-actions github-actions bot added the platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. label Oct 27, 2023
@ghost
Copy link

ghost commented Oct 27, 2023

👀 Review this PR in a CodeSee Review Map

View the CodeSee Map of this change

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@huyenltnguyen huyenltnguyen self-assigned this Oct 27, 2023
@huyenltnguyen huyenltnguyen added the status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. label Oct 27, 2023
client/src/pages/email-sign-up.css Outdated Show resolved Hide resolved
client/src/pages/email-sign-up.tsx Outdated Show resolved Hide resolved
@huyenltnguyen huyenltnguyen added status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP and removed status: blocked Is waiting on followup from either the Opening Poster of the issue or PR, or a maintainer. labels Nov 3, 2023
@huyenltnguyen
Copy link
Member

@JeevaRamanathan The CI failure has been resolved and the fix was merged into main, so please help update your branch with the latest main 🙂

Copy link
Member

@huyenltnguyen huyenltnguyen left a comment

Choose a reason for hiding this comment

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

@JeevaRamanathan thank you for the PR and for bearing with me!

I've tested locally using the combination of Row and Col from both ui-components and @freecodecamp/react-bootstrap, and I didn't see a gap between the footer and the edge of the screen.

Here is the result (tested on both Chrome and Firefox):

Desktop Mobile
Screenshot 2023-11-06 at 16 10 40 Screenshot 2023-11-06 at 16 07 39

With that, I updated the PR to switch back to the Row component. Let's wait for another maintainer to help us test and give the final verdict 😄

@huyenltnguyen huyenltnguyen added status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending. and removed status: waiting update To be applied to PR if a maintainer/reviewer has left a feedback and follow up is needed from OP labels Nov 6, 2023
@JeevaRamanathan
Copy link
Contributor Author

JeevaRamanathan commented Nov 6, 2023

@huyenltnguyen Sorry for dragging this conversation further😅, but I still have a doubt that I'd like to clarify.
So in the screenshot that you have shared above, if you see that profile icon it's partially hidden which is also causing that space in the footer.
image

And if you see the below screenshot (settings screen), its having equal margins on save button and also profile icon is clearly visible.
image.

So is that expected to have no margins in Yes/No button in email-sign-up screen or could you please clarify?

@huyenltnguyen
Copy link
Member

Okay, you got me. I noticed the profile image cutoff / horizontal scroll, too. But I believe it has been like that for a while (I got the same issue when tried using the bootstrap components), so I didn't want to address that in this PR to avoid scope creep.

Now that you mentioned it, I went ahead and fixed the issue by wrapping the button block inside a Container, which helps cancel out the negative margins. Whether this is necessary or not will be determined when we revisit the component library, but this is currently how the other pages / most of the pages are implemented.

Screenshots of the result (tested on Chrome and Firefox):

Desktop Mobile
Screenshot 2023-11-06 at 18 17 22 Screenshot 2023-11-06 at 18 17 48

@raisedadead raisedadead merged commit 281baf7 into freeCodeCamp:main Nov 6, 2023
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. status: waiting review To be applied to PR's that are ready for QA, especially when additional review is pending.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Alignment Issue: Button in Email Sign-up Section
3 participants