Skip to content

Readjust narrow desktop layout#1045

Open
Thirukumaran-T wants to merge 8 commits into
falling-fruit:mainfrom
Thirukumaran-T:readjust-narrow-desktop-layout
Open

Readjust narrow desktop layout#1045
Thirukumaran-T wants to merge 8 commits into
falling-fruit:mainfrom
Thirukumaran-T:readjust-narrow-desktop-layout

Conversation

@Thirukumaran-T
Copy link
Copy Markdown
Contributor

@Thirukumaran-T Thirukumaran-T commented May 5, 2026 •
edited by wbazant
Loading

Closes #980

@Thirukumaran-T
Copy link
Copy Markdown
Contributor Author

Thirukumaran-T commented May 5, 2026

Please review this PR! @wbazant ..

wbazant
wbazant requested changes May 5, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@wbazant wbazant left a comment

Choose a reason for hiding this comment

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

Thanks, I think the approach will work! I reviewed the code and requested some small changes.A screencast of the header opening in various widths would help me review the resulting UI if its not hard for you to make.

Comment thread src/components/desktop/Header.js Outdated
@@ -250,6 +283,7 @@ const SignupButton = styled(Button)`
const StyledSocialButtons = styled(SocialButtons)`
margin: 0 5px;
display: flex;
Copy link
Copy Markdown
Collaborator

@wbazant wbazant May 5, 2026

Choose a reason for hiding this comment

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

Can we remove this line?

Copy link
Copy Markdown
Collaborator

@wbazant wbazant May 5, 2026

Choose a reason for hiding this comment

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

The display is redefined below.

Comment thread src/components/desktop/Header.js
Comment thread src/components/desktop/Header.js Outdated
const StyledSocialButtons = styled(SocialButtons)`
margin: 0 5px;
display: flex;
display: ${({ user }) => (layoutIsVeryNarrow(user) ? 'none' : 'flex')};
Copy link
Copy Markdown
Collaborator

@wbazant wbazant May 5, 2026

Choose a reason for hiding this comment

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

I've not yet ran the code, but will this get hidden always or only if the menu is open? We want them available at least sometimes.

@Thirukumaran-T
Copy link
Copy Markdown
Contributor Author

Thirukumaran-T commented May 7, 2026

I'll do the required changes and update!

@Thirukumaran-T
Copy link
Copy Markdown
Contributor Author

Thirukumaran-T commented May 7, 2026 •
edited
Loading

I have updated the PR @wbazant .. please review and i'll upload the pictures
one
below
two
three

@wbazant
Copy link
Copy Markdown
Collaborator

wbazant commented May 7, 2026

Thankjs for making the changes! I'm back from holiday and I have my laptop back, so I checked out the branch and tried different widths. I think your 'very narrow' layout needs a fix - it hides the social buttons when they don't need hiding, and there's still an overflow:
Screencast from 2026-05-07 21-01-55.webm

@Thirukumaran-T
Copy link
Copy Markdown
Contributor Author

Thirukumaran-T commented May 8, 2026 •
edited
Loading

@wbazant I have refacored the code for very narrow layout and the social buttons are only hiding when truly necessary and the overflow.

Hope you had a nice holiday! 🌴.

@wbazant
Copy link
Copy Markdown
Collaborator

wbazant commented May 8, 2026

This is not the right fix, I would like the social buttons to be hidden in this situation:

Screenshot from 2026-05-08 10-19-43

@Thirukumaran-T
Copy link
Copy Markdown
Contributor Author

Thirukumaran-T commented May 8, 2026

@wbazant i have refactored the code for very narrow layout.. please review

@wbazant
Copy link
Copy Markdown
Collaborator

wbazant commented May 9, 2026

No, that change does not work.

@Thirukumaran-T
Copy link
Copy Markdown
Contributor Author

Thirukumaran-T commented May 11, 2026 •
edited
Loading

Can you please suggest me a fix..while testing i have seen for a very narrow layout the social icons is not visible.. @wbazant is it still overflowing!

@wbazant
Copy link
Copy Markdown
Collaborator

wbazant commented May 11, 2026

Sure, happy to clarify what the problem is here! By the overflow I mean, the menu part of the ribbon doesn't fit in the width it has and the browser tries to squish it in, which looks like the little triangle that is usually to the right of the 'About' menu getting its own line. Your change doesn't address the problem, because you're removing the social buttons when the menu is not visible, instead of when it's visible. I think if you fix that by changing the logic a bit, remove the CSS changes you introduced while trying to solve the problem, and test again in your browser, the PR will be good to go.

@Thirukumaran-T
Copy link
Copy Markdown
Contributor Author

Thirukumaran-T commented May 18, 2026

Okk!.. i'll do that required changes

@Thirukumaran-T
Copy link
Copy Markdown
Contributor Author

Thirukumaran-T commented May 23, 2026

@wbazant can you review my changes once

@wbazant
Copy link
Copy Markdown
Collaborator

wbazant commented May 23, 2026

I looked at the code change and it doesn't look like it addresses what's needed - we want the social buttons visible at the start and hidden once the menu button is pressed, so the menu content has enough space. Your change instead hides them at the start and makes them visible once the menu button is pressed, so the menu content doesn't gain any additional space. Also, sorry, I see you're going in circles on this and I'm not really sure how to unblock you - maybe starting again and making sure you can reproduce the issue in your browser before making any changes, or maybe I will take this issue later and you could do the new translation into Tamil instead?

@Thirukumaran-T
Copy link
Copy Markdown
Contributor Author

Thirukumaran-T commented May 23, 2026 •
edited
Loading

@wbazant i'll try it once.. if it is not resolved you can take this issue and i'll move to translation

@Thirukumaran-T
Copy link
Copy Markdown
Contributor Author

Thirukumaran-T commented May 23, 2026

@wbazant have you reviewed it!..

@wbazant
Copy link
Copy Markdown
Collaborator

wbazant commented May 23, 2026

@Thirukumaran-T I've not run it but I looked at the code diff, it has a bunch of changes, I imagine you kept prompting an LLM when I was telling you your changes don't work. I also realised we might actually just need two lines:

+ const VERY_NARROW_LAYOUT_WIDTH = ...

        <div className="auth-social">
          {!isMobileMenuOpen && <UserMenu />}
-           <StyledSocialButtons />
+        {(!isMobileMenuOpen || window.innerWidth > VERY_NARROW_LAYOUT_WIDTH ) &&  <StyledSocialButtons /> }

Feel free to try that on a fresh checkout and see if it hides the buttons when the menu is open, so the menu has enough space in a narrow layout? Can you experimentally find a good value for VERY_NARROW_LAYOUT_WIDTH?

@Thirukumaran-T
Copy link
Copy Markdown
Contributor Author

Thirukumaran-T commented May 24, 2026

I have understood the issue but it seems overwork for it.. i have to learn more about this i'll move to tamil translation can you take over this issue and raise PR .. i'll learn from your PR what has to be really changed

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

Reviewers

@wbazant wbazant wbazant requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Readjust the narrow desktop layout so there's no overflow after opening the menu

2 participants

@Thirukumaran-T @wbazant