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

Make part of next color picker visible in color pickers scroll bar #17443

Closed
pavloburykh opened this issue Sep 28, 2023 · 11 comments · Fixed by #17465 or #17748
Closed

Make part of next color picker visible in color pickers scroll bar #17443

pavloburykh opened this issue Sep 28, 2023 · 11 comments · Fixed by #17465 or #17748
Assignees

Comments

@pavloburykh
Copy link
Contributor

Followup of #17405 cc @OmarBasem

Discussed here

Currently part of next color picker is not visible wich makes it not obvious for user that color pickers are scrollable.

We should make part of next color picker visible as it is shown on the designs

Expected result:

https://www.figma.com/file/o4qG1bnFyuyFOvHQVGgeFY/Onboarding-for-Mobile?type=design&node-id=2241-250931&mode=design

Onboarding for Mobile – Figma 2023-09-28 11-35-17

Actual result:

photo_2023-09-28 11 07 33

Additional Information

  • Status version: nightly
  • Operating System: Android, iOS
@ibrkhalil
Copy link
Contributor

Hey @pavloburykh
This is how it looks like for me on 14 Pro emulator
image
May I know what device you're using for testing this?

@pavloburykh
Copy link
Contributor Author

May I know what device you're using for testing this?

Hi @ibrkhalil
This is how it looks for me

iPhone X:

photo_2023-09-28 11 07 33

Samsung Galaxy A52:

photo_2023-09-28 11 07 35

@ibrkhalil
Copy link
Contributor

ibrkhalil commented Oct 8, 2023

Thinking more about implementing it, I think it's not possible to make the same look on all devices while having each circle the same width because devices have different widths and the number of visible circles and how they look is very much dependant on device width.
So I'd ask permission from Design to allow me to make the widths of the circles dynamic (By finding their ratio for iPhone 11 Pro and applying that for all devices)
CC: @Francesca-G
I used ratios in my branch and this how it looks like with a ratio applied instead of values.

CleanShot 2023-10-08 at 19 51 44

@OmarBasem
Copy link
Contributor

@ibrkhalil we can make the margins dynamic

@ibrkhalil
Copy link
Contributor

@ibrkhalil we can make the margins dynamic

Let me give it another attempt with dynamic margins and report back.
I tried it already yesterday but couldn't get it to look good enough.
Maybe I missed something..

@OmarBasem
Copy link
Contributor

@ibrkhalil A circle's total size is its width plus its margin. So making the width dynamic or the margin dynamic should give the same result

@Francesca-G
Copy link

Thinking more about implementing it, I think it's not possible to make the same look on all devices while having each circle the same width because devices have different widths and the number of visible circles and how they look is very much dependant on device width. So I'd ask permission from Design to allow me to make the widths of the circles dynamic (By finding their ratio for iPhone 11 Pro and applying that for all devices) CC: @Francesca-G I used ratios in my branch and this how it looks like with a ratio applied instead of values.

As mentioned here we shouldn't change the circle size but margins between circles should be flexible :)

@ibrkhalil
Copy link
Contributor

Ookay, Let me give it a shot.

@cammellos
Copy link
Contributor

@ibrkhalil could you give us an update on this one please?

@ibrkhalil
Copy link
Contributor

@ibrkhalil could you give us an update on this one please?

I mentioned @pavloburykh on the PR, Telling him that I did the dynamic margin and it's ready for a rereview

@pavloburykh
Copy link
Contributor Author

I mentioned @pavloburykh on the PR, Telling him that I did the dynamic margin and it's ready for a rereview

Thank you. I will review today and provide feedback in the PR.

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