Skip to content

Conversation

@AprilSylph
Copy link
Owner

@AprilSylph AprilSylph commented Dec 19, 2025

Description

Before Before After After
Screen Shot 2026-01-05 at 09 30 37 Screen Shot 2026-01-05 at 09 32 10 Screen Shot 2026-01-06 at 09 41 27 Screen Shot 2026-01-06 at 09 41 08

This colour picker looks more modern and does not have any major security vulnerabilities (see lack of comments on this PR from CodeQL). Might also improve touchscreen compatibility? When I try to use Spectrum with Firefox simulating a touchscreen, the picker doesn't stay open...

While I was here, I added some Tumblr brand colours as colour picker shortcuts. Spectrum supports this as well, but I just haven't had a reason to touch our implementation until now.

Testing steps

  1. Load the modified addon
  2. Enable Painter
  3. Try to configure Painter
    • Expected result: It is possible to select a brand colour via the swatch shortcuts
    • Expected result: It is possible to select a custom colour via the picker
    • Expected result: It is possible to use TAB to focus the text inputs directly
  4. Open a Tumblr tab
    • Expected result: Painter activates on posts as configured
  5. Exploit TAB to configure Painter incorrectly (i.e. type a non-colour string into a colour input)
    • Expected result: Nothing you do can cause Painter to have adverse effects on posts
      (If a non-colour would be present in the constructed gradient, the gradient simply doesn't get applied, leaving an uncoloured border on the top of the affected posts)

@AprilSylph AprilSylph marked this pull request as ready for review January 5, 2026 10:05
@marcustyphoon
Copy link
Collaborator

Does it look good on ipad? "No, but we have bigger problems there" :D

@marcustyphoon
Copy link
Collaborator

Tested and functions in:

  • Chromium 105
  • Firefox 128
  • Safari 18, iPad simulator
  • Safari 18, iPhone

@marcustyphoon
Copy link
Collaborator

If you filter the preference pane to just show painter and open the final color picker, the preference pane extends lower so that the color picker's full height is accessible by scrolling down; this is good. Dismissing the color picker scrolls you back up, and opening it again leaves you scrolled up; this is fine.

Something we could do if desired is enforcing a minimum height for the configuration pane, making it appear that the search option filters the feature items down atop a—surface—okay, yeah, I don't know the UI design terminology for this. Anyway, that would probably feel better in this specific case, but I don't know that it would be worth it, as in most cases it probably feels more natural not to see the surface(?) behind the feature items except in the "no results found" case.

@AprilSylph
Copy link
Owner Author

If you filter the preference pane to just show painter and open the final color picker, the preference pane extends lower so that the color picker's full height is accessible by scrolling down; this is good. Dismissing the color picker scrolls you back up, and opening it again leaves you scrolled up; this is fine.

Something we could do if desired is enforcing a minimum height for the configuration pane, making it appear that the search option filters the feature items down atop a—surface—okay, yeah, I don't know the UI design terminology for this. Anyway, that would probably feel better in this specific case, but I don't know that it would be worth it, as in most cases it probably feels more natural not to see the surface(?) behind the feature items except in the "no results found" case.

Hmm, the jump is a little weird now that you point it out... but this is definitely out of scope for this PR. Feel free to open an enhancement issue if you want. (If you don't, I will probably forget about this completely by the end of the day!)

@AprilSylph
Copy link
Owner Author

Does it look good on ipad? "No, but we have bigger problems there" :D

If you have any idea how to force the popup to be wider on mobile Safari without messing up the semi-auto width logic on desktop Firefox/Chrome, please please PR it, because this screenshot makes me sad.

@AprilSylph AprilSylph merged commit 6e0c6bc into master Jan 6, 2026
5 checks passed
@AprilSylph AprilSylph deleted the aprilsylph/coloris branch January 6, 2026 09:46
@marcustyphoon
Copy link
Collaborator

I never bothered to understand how the width logic worked normally, so I don't :D Maybe someday I'll just throw stuff at the wall until something sticks.

@AprilSylph
Copy link
Owner Author

Ah, that's fair, I did just throw stuff at the wall until I understood it too...

html, main {
width: 375px;
min-width: 100%;
max-width: 100%;
}

  • width declaration: sets the intrinsic width, so that the descendants don't matter for width calculation (e.g. Show Originals' description isn't forced to sit on one unbroken line), and gives us a sane default; in the absence of viewport constraints, have a 375px content box—this is the only relevant rule when opening the popup via browser action on desktop Firefox/Chrome.
  • min-width declaration: if the viewport is over 375px wide, grow to fill the space. Without this, the UI wouldn't be full-width when opened in a new tab or about:addons or chrome://extensions.
  • max-width declaration: if the viewport is under 375px wide, shrink to fit so that there isn't a horizontal scrollbar. Now that Firefox doesn't allow pinning extensions to the overflow menu, this is only relevant when opening the UI on a mobile device.
  • Using html, main instead of just html or main: I don't remember for sure, I think this is just for browser compat—but either way it syncs the width of the content with the width of the document, which is what we want.

I'm guessing that iPad Safari's popup viewport has its own intrinsic width that means we shrink to fit it. Question is, would the popup viewport width grow if we didn't? And how would we reliably detect iPadOS to unset the max-width rule? We could check the user agent, of course, but I don't like how non-deterministic that is. Although, in practice, "detect iPad, assume problem, fix problem" is probably exactly the same behaviourally as "detect problem, fix problem".

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.

replace Spectrum plugin for colour picking

3 participants