fix(Whiteboard): broken color picker in RTL layout
Purpose / Description
Replace the existing color picker library (inactive since last 4 years) with another, fairly up-to-date library.
Fixes
- Fixes: #19237
Approach
- Create a utility function to create a color picker dialog with appropriate defaults
- Replace the usage of
com.mrudultora.colorpicker.ColorPickerPopUpwith the equivalent new colorPicker
https://github.com/user-attachments/assets/3e86f0c0-d58e-4a49-82a8-091825d72099
Before
After
How Has This Been Tested?
Manually tested on Poco X3 NFC
Checklist
- [x] You have a descriptive commit message with a short title (first line, max 50 chars).
- [x] You have commented your code, particularly in hard-to-understand areas
- [x] You have performed a self-review of your own code
- [x] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
- [ ] UI Changes: You have tested your change using the Google Accessibility Scanner
[!IMPORTANT] Maintainers: This PR contains https://github.com/ankidroid/Anki-Android/labels/Strings changes
- Sync Translations before merging this PR and wait for the action to complete
- Review and merge the auto-generated PR in order to sync all user-submitted translations
- Sync Translations again and merge the PR so the huge automated string changes caused by merging this PR are by themselves and easy to review
Learning: NEVER change your phone's language to something you don't understand ☠️☠️
The drawing screen is also updated right? (Note editor -> clip icon -> Drawing)
If I start the color selection dialog and don't change at all the brightness then I get black as the color no matter what I select.
@lukstbit I don't get what you mean? I tried doing so, I didn't get black as color. I opened color-selection dialog and only changed color (no change to alpha/brightness).
https://github.com/user-attachments/assets/01334489-003f-4dbb-8a6f-28b33d07b40c
@lukstbit I don't get what you mean? I tried doing so, I didn't get black as color. I opened color-selection dialog and only changed color (no change to alpha/brightness). Screen_recording_20251103_215049.mp4
I can see the behavior I mentioned on both reviewers. It would be helpful if someone else can confirm/infirm what I'm seeing:
Wow, weird! Will inspect it further, trying with different device. Difference I see here is light theme, will try once with light theme too.
Reproduced with a light theme, but not with a dark one.
This issue only happens when initially selected color is "Black". When the initially selected color is black, color picker's anchor is at center (White point) and brightness is at max.
Stuck at resolving the invalid color value, looks like a bug upstream related to conversion between ARGB and HSV pallets.
You also need to fill out the Licenses section under the PR description template
Alpha now set to correct position after setInitialColor
Picked bubble shows color with alpha
Almost!!
The first color picker in your example could be confused for being a selection marker, as the selected color and outer color are the same.
Could you try a minimal circular border (1px white) between the current color, and the outer color of the selection marker?
Hmm... Lemme try!
@david-allison Looks good?
I had to import the svg into figma and put a white circle, adjust and play around the dimensions to be concentric with color-tile.
Do you think there's a easier way to do that in XML?
In compose I see a very clear way just to add a modifier if I want to draw something, I can go crazy and draw anything with canvas behind any ui-node.
Android Vector XML isn't my strong suit.
I'd presume you can just place a circle of radius n+1 underneath it
Yeah, that's what I did, in actual svg though.
Checked it out. Design-wise: yes.
Latency-wise: there's noticeable lag between the position updates of the under-cursor icon and position marker.
The latency is noticeable when moving fast. Reminds me of the tests done by MS: https://www.youtube.com/watch?v=vOvQCPLkPt4
I also managed to end up with this situation when I released my mouse button
Latency-wise: there's noticeable lag between the position updates of the under-cursor icon and position marker.
Is it noticeable enough to bother a user? I tried on my device, physical and emulator. I did see some lag between my mouse movement and cursor but it din't bother me. I'd see this as a problem if I were drawing something.
I also managed to end up with this situation when I released my mouse button
The bubble tries to stay within the bounds of color wheel, while showing the color under the pointer. In general scenario too, bubble's tails stays at circumference of the circle, but the color is one at the center of pointer.
https://github.com/user-attachments/assets/4964b07b-0b03-4dd3-83a3-c2f1b86014b6
Is it noticeable enough to bother a user? I tried on my device, physical and emulator.
Brayan's video (mildly) bothers me due to the latency
EDIT: if this is an involved fix, we need to make the judgment call: "is this better than the past functionality, and can we add appropriate TODOs"
latency is similar to before
https://github.com/user-attachments/assets/3f91a1a1-3506-4fad-bbfb-237f98508520
Latency of the new color selection cursor updates- it's not aligned with the 'touch' cursor. Screenshot from a video I made:
It's expected that they're not perfectly in sync with the user's touch, but they should be aligned with each other
Thanks @david-allison Will apply the patch the push this week :) Time to learn more about Choreographer API, haven't yet looked at it!