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

feat: custom locale switcher #2981

Merged
merged 4 commits into from
Jan 30, 2024
Merged

feat: custom locale switcher #2981

merged 4 commits into from
Jan 30, 2024

Conversation

pavanjoshi914
Copy link
Contributor

@pavanjoshi914 pavanjoshi914 commented Jan 3, 2024

Describe the changes you have made in this PR

local switcher in onboardings screen
custom design to use it diffferent places

Link this PR to an issue [optional]

Fixes #2918

Type of change

(Remove other not matching type)

  • feat: New feature (non-breaking change which adds functionality)

Screenshots of the changes [optional]

image

image

Checklist

  • Self-review of changed code
  • Manual testing
  • Added automated tests where applicable
  • Update Docs & Guides
  • For UI-related changes
  • Darkmode
  • Responsive layout

@@ -64,7 +65,10 @@ function Layout() {
return (
<div className="flex justify-center items-center min-h-screen">
<div className="w-full">
<div className="max-w-7xl mx-auto px-4 sm:px-6 lg:px-8 mb-4">
<div className="absolute top-0 left-0 mt-4 ml-6 z-50">
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't use absolute positioning or add some proper paddings on the outer element that prevent overflowing on smaller screens.

image

Maybe center the dropdown on smaller screens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

@pavanjoshi914
Copy link
Contributor Author

true absolute positioning was a bad idea there. cause even if we scroll switcher will be visible and designs are not like that to support this. i changed layout with something else

@stackingsaunter
Copy link
Contributor

Can't test without built but tACK

Copy link
Contributor

@reneaaron reneaaron left a comment

Choose a reason for hiding this comment

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

Now the layout exceeds the window height and I see scrollbars (although there shouldn't be any).

@pavanjoshi914
Copy link
Contributor Author

Now the layout exceeds the window height and I see scrollbars (although there shouldn't be any).

I am unclear about the behavior we want to achieve here! it really depends on the content as well right? for eg. if i surf extension in brasil lang i can see scrollbars. as text there is longer.

locale switcher takes addditional space. even we adjust some content for en lang on other contents there will be still the scrollbars appearing

@pavanjoshi914
Copy link
Contributor Author

video_2024-01-23_12-46-06.mp4

@reneaaron
Copy link
Contributor

Sorry, I should have added a screenshot. The fullscreen page now always has scrollbars since the content of the page exceeds the available height.

image

@pavanjoshi914
Copy link
Contributor Author

resolved

Copy link
Contributor

@reneaaron reneaaron left a comment

Choose a reason for hiding this comment

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

tACK

@reneaaron reneaaron merged commit bcab944 into master Jan 30, 2024
6 of 7 checks passed
@reneaaron reneaaron deleted the onboard-locale-switcher branch January 30, 2024 22:08
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.

Add language picker to the onboarding
3 participants