Skip to content

feat: Improve language picker UI with icons - #1933

Closed
ShubhamOulkar wants to merge 18 commits into
expressjs:gh-pagesfrom
ShubhamOulkar:enhance-lang-picker
Closed

feat: Improve language picker UI with icons#1933
ShubhamOulkar wants to merge 18 commits into
expressjs:gh-pagesfrom
ShubhamOulkar:enhance-lang-picker

Conversation

@ShubhamOulkar

@ShubhamOulkar ShubhamOulkar commented May 30, 2025

Copy link
Copy Markdown
Member

Task

closes #1804

new design applied from https://expressjsnewdesign.netlify.app/

Design

(Following design rejected)

desktop dark desktop light
image image

bug: Mobile view, current language color highlighting

image

How SVG Icons Were Created?

  • The SVG flag icons were sourced from Nucleo SVG Flag Icons.
  • Each SVG file was copied and added to the repository using the VS Code editor.
  • While there are several ways to edit SVGs in VS Code, including using extensions for live preview or advanced editing, I used the default text editor setup to make the required changes.
  • The SVGs were customized and optimized to fit the required sizes(20x22) and design specifications for our language picker UI, so that we don't need to write CSS to customize icons.

@ShubhamOulkar
ShubhamOulkar requested review from a team as code owners May 30, 2025 14:51
@netlify

netlify Bot commented May 30, 2025

Copy link
Copy Markdown

Deploy Preview for expressjscom-preview ready!

Name Link
🔨 Latest commit 75031e4
🔍 Latest deploy log https://app.netlify.com/projects/expressjscom-preview/deploys/687873d6d9529f0008a91091
😎 Deploy Preview https://deploy-preview-1933--expressjscom-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@ShubhamOulkar ShubhamOulkar self-assigned this May 30, 2025
@ShubhamOulkar ShubhamOulkar changed the title WIP : Enhance lang picker feat: Improve language picker UI with icons May 31, 2025
Comment thread js/menu.js Outdated
Comment thread js/menu.js Outdated
Comment thread css/variables.css Outdated
Comment thread _includes/icons/flags/de.svg Outdated

@ShubhamOulkar ShubhamOulkar left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

LGTM

@ShubhamOulkar
ShubhamOulkar requested a review from bjohansebas June 4, 2025 05:09
@cengizcmataraci

Copy link
Copy Markdown
Contributor

cool work, thanks for your contribution.

@ShubhamOulkar
ShubhamOulkar requested a review from a team as a code owner June 12, 2025 13:20
@ShubhamOulkar
ShubhamOulkar removed the request for review from a team June 12, 2025 13:24

@ShubhamOulkar ShubhamOulkar left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi @bjohansebas, we can launch this pr. Flags rendering logic is changed in c37b3a4

LGTM 👍, I need approvals to launch these changes.

@ShubhamOulkar ShubhamOulkar added enhancement design Change, update, or fix for site UI (not content) labels Jun 12, 2025
Comment thread CONTRIBUTING.md Outdated

@ctcpip ctcpip left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should not conflate languages with countries for a number of reasons (different languages spoken in the same country, people who speak a language may not identify with the country we have chosen, etc.)

While it's true that we sometimes need to choose a regional variation of a language for machine translation purposes, e.g. Brazilian Portuguese, that doesn't mean we must represent it using a Brazilian flag.

@ShubhamOulkar

ShubhamOulkar commented Jul 7, 2025

Copy link
Copy Markdown
Member Author

Thanks @ctcpip for you time. My initial thoughts were same as yours. Let's wait for others, since it was proposed in issue #1804. Otherwise I am ok with removing icons and move on with just bug fixes as reported in description.

@bjohansebas

Copy link
Copy Markdown
Member

I agree with @ctcpip

Comment thread _includes/icons/translation-global.svg Outdated
Comment thread CONTRIBUTING.md Outdated
@bjohansebas

Copy link
Copy Markdown
Member

Can we leave just the icon, please?
imagen

@ShubhamOulkar

Copy link
Copy Markdown
Member Author

Can we leave just the icon, please? imagen

Actually, I want to 😃. This approach removes a lot of unnecessary files and code. We don't need mobile lang picker file as well as JS and CSS written for this. I already mentioned this in comment #1804 (comment). But didn't got reviews. Then I decide to keep as it is and just fix issues referenced in description. Also I did check history, you guys did a lot of work to bring that mobile lang picker on the website.

@ShubhamOulkar

Copy link
Copy Markdown
Member Author

Long story short 😃 — on small screens, we only show the i18n icon, and now we're switching desktop to use the same. So it makes sense to use a single UI component across all screen sizes.

@ShubhamOulkar
ShubhamOulkar marked this pull request as draft July 21, 2025 14:26
@ShubhamOulkar

Copy link
Copy Markdown
Member Author

I'm going to draft this PR for now. It will be closed later as part of the i18n component simplification, as mentioned in #1933 (comment).

@ShubhamOulkar
ShubhamOulkar deleted the enhance-lang-picker branch August 2, 2025 11:32
@ShubhamOulkar

Copy link
Copy Markdown
Member Author

We decided to make design simple and reusable on all screen sizes.

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

Labels

design Change, update, or fix for site UI (not content) enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Language picker: Add identifying feature and remove/fix duplicate nav ids

4 participants