Skip to content

Conversation

@Chromoprotein
Copy link
Contributor

📄 Pull Request Overview

closes 211

🔧 Changes Made

  1. Made a custom drop-down select menu for switching languages.

  2. Added the requested style improvements: aligned the text to left, made the arrow smaller with more spacing, improved the border's style cohesion, and made the mobile version look cleaner.


Checklist Before Submission

  • Functionality: I have tested my code, and it works as expected.
  • JSDoc: I have added or updated JSDoc comments for all relevant code.
  • Debugging: No console.log() or other debugging statements are left.
  • Clean Code: Removed commented-out or unnecessary code.
  • Tests: Added new tests or updated existing ones for the changes made.
  • Documentation: Documentation has been updated (if applicable).

📝 Additional Information

custom-menu

@Skoivumaki
Copy link
Member

@Chromoprotein is this a new PR? Did you do requested changes from the last one?

@Chromoprotein
Copy link
Contributor Author

@Skoivumaki Yes, sorry I made a new branch and PR because I had some git problems. I made the requested changes except the animation. Is it ok if that's a separate issue?

@Skoivumaki
Copy link
Member

@Chromoprotein Did you figure out the ident issue?

Is it ok if that's a separate issue?

Maybe if needed

@Chromoprotein
Copy link
Contributor Author

@Skoivumaki I noticed the mobile version was making the logout button's text next to it look misaligned, so I fixed that, but I'm not sure that was the problem tbh.

@Skoivumaki
Copy link
Member

@Skoivumaki I noticed the mobile version was making the logout button's text next to it look misaligned, so I fixed that, but I'm not sure that was the problem tbh.

image
Notice how the button used to look. Now that the border matches the size of "log out", the spacing seems too much imo. I think this is what was meant with ident. I would reduce spacing between text and svg to not make the button seem so wide. Only for desktop, mobile is fine as is.

@Skoivumaki
Copy link
Member

@leolabdev clarification about the ident thing?

@leolabdev
Copy link
Member

leolabdev commented Dec 12, 2024

@Skoivumaki @Chromoprotein
It looks better, but I don't understand why the margins look different on mobile and desktop sizes.

image
image

VS

image
image

Also, on the browser solution, you can see that the option and label are on the same level and aligned to the left. I think we should also stick to this. Also, now when hovering over it, this is not shown to the user at all.

Copy link
Member

@leolabdev leolabdev left a comment

Choose a reason for hiding this comment

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

see my comments above

@leolabdev leolabdev added the enhancement For improvements to existing features label Dec 12, 2024
@leolabdev leolabdev added this to the UI/UX milestone Dec 12, 2024
@Chromoprotein Chromoprotein force-pushed the pauliina/enhancement/211-make-custom-select-menu branch from 7732394 to 5bc139c Compare December 16, 2024 21:29
@Chromoprotein
Copy link
Contributor Author

langswitcher changes

How does it look now? The margins are the same on mobile and desktop, the option and label are on the same level and aligned to the left, and hovering is indicated by the text color.

@Jonroi Jonroi requested a review from a team January 16, 2025 09:31
Copy link
Contributor

@Jonroi Jonroi left a comment

Choose a reason for hiding this comment

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

I think this implementation works well enough for now—it functions properly on both desktop and mobile. Let’s move forward with it as-is, since our next step will involve a complete navbar redesign.

Screenshots:

Desktop View:

Desktop View

Mobile View:

Mobile View

@Jonroi Jonroi requested a review from a team January 16, 2025 15:33
@Chromoprotein
Copy link
Contributor Author

@leolabdev It looks like this can't be merged until you approve it, could you please review? 🙂

@Jonroi Jonroi merged commit c67e3d1 into dev Jan 26, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement For improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants