ColorPicker: Allow commandStateStore to handle background-color

[ethan-james]

Fixes #3903, #3904 and #3905

It seems that removing the background-color regex fixes this issue pretty effectively. Allowing commandStateStore to track the state of formatting at the current caret position means that selected will be true while the caret is inside of a formatting tag, and false while it is outside. This may lead to some unexpected behavior and I will follow up with examples next week, but I feel that this behavior is better addressed in commandStateStore or possibly in formatSelection rather than through use of this additional regex.

I tried to remove the textHexColor regex as well, but I started to see issues with selected being incorrect, so I figure that we can come back to that if this approach seems to be working.

It seemed to work fine in the browser, but the Puppeteer test was showing different behavior until I made a change to the getCommandState util. It was matching on any font or span tag within the text that it was analyzing, and I don't think it makes sense to match foreColor or backColor unless it wraps the entire thought. It is also possible to have the thought wrapped in other formatting tags (<b>, <i>, etc.) so I updated the regex to allow any of those tags at the beginning of the matched text, but to reject any text other than a formatting tag in front of the font or span.

I did test this against #1957 and #2267 to make sure I wasn't creating any regressions. Let me know if there are other issues associated with this functionality that I should check out.

[ethan-james]

Here is an example of unexpected behavior that was not possible before fixing this issue, because it wasn't possible to highlight two different sections of a thought using the same color. I recommend that we create a new issue for this.

Steps to Reproduce

1. Create a thought "hello there world"
2. Double click "hello"
3. Set background color
4. Double click "world"
5. Set same background color
6. Single click "hello"
7. Toggle background color (click same background color again)

Expected behavior

Either it adds the background color to the entire thought (ignores caret position within formatted text) or else it removes the formatting tag that immediately surrounds the caret (caret-aware command state)

Current behavior

Removes background color from all selections that have that same color applied

Recording.2026-04-07.173429.mp4

[ethan-james]

I'm going to add /skip-tdd to the unit tests for the reason described above (changed them from a valid-but-possibly-outdated format to a valid-and-standard format) but I noticed a couple of things:

1. TDD Puppeteer tests seem to be missing action.yml https://github.com/cybersemics/em/actions/runs/24164197763/job/70522175765?pr=4034
2. Adding /skip-tdd as a comment in __tests__/getCommandState.ts seems to skip all TDD tests, while I was just trying to skip the unit tests specifically

[ethan-james]

This fixes #3905 as well, so I added a test to cover the case of undo restoring previous background colors.

[raineorshine]

This text is a bit confusing, since the thought is not tapped in this test case, and the length of the thought does not appear to be relevant. Maybe the text was from a different example?

[raineorshine]

Simpler:

await paste(`...`)

[raineorshine]

because it's not possible to create separate tags for foreground and background color using any method that I know.

It is possible to paste valid HTML with separate tags for foreground and background color. Our import logic does not currently have the ability to merge or normalize pasted tags. Also, different platforms may yield different HTML when formatting selected text, so I can't be 100% sure this is safe across iOS and Android. So, it would be best if we could support separate tags.

[ethan-james]

I think that if the regex is getting any more complex, then we're better off replacing it with DOMParser. I tried it out on desktop Chrome and iOS Safari and it seems to work the same, and it works OK in the unit tests.

[raineorshine]

It seems I released the new TDD workflow prematurely. Thanks for your patience as I roll out this new part of the CI!

• TDD Puppeteer tests seem to be missing action.yml cybersemics/em/actions/runs/24164197763/job/70522175765?pr=4034

I will look into this. I'm not sure what it means, since action.yml exists.

• Adding /skip-tdd as a comment in __tests__/getCommandState.ts seems to skip all TDD tests, while I was just trying to skip the unit tests specifically

These were supposed to be PR comments, not in-code comments, so that was another big oversight. Test code should never be permanently exempt from TDD.

I'll add /skip-tdd-puppeteer commands for more granular control.

[ethan-james]

Quick update on this issue: it appears that the toolbar buttons are not being triggered by the current implementation in tap.ts (iOS helper).

At first, I suspected this might be due to the “Text Color” icon not being visible on mobile, requiring a scroll action before interaction. However, further testing showed that even toolbar icons already visible on screen are not responding to the tap actions. I also explored a few alternative APIs from WebdriverIO, but none seem to work.

As a workaround, I implemented a helper function that directly interacts with the DOM. It waits for the target element using the appropriate selector and then dispatches mouse events programmatically:

node.dispatchEvent(new MouseEvent('mousedown', { button: 0 }))
node.dispatchEvent(new MouseEvent('mouseup', { button: 0 }))
node.click()

This approach resolves the issue functionally. That said, it bypasses the standard user interaction flow by triggering internal DOM events rather than simulating an actual user tap, so it doesn’t feel like the most robust or ideal long-term solution.

I would love to hear some ideas on this! @raineorshine @ethan-james

Thanks @karunkop! Did you get it to work locally, or in another branch? I tried my hand at it and ended up with

2026-05-05T21:27:10.5930819Z unknown error: WebDriverError: An unknown server-side error occurred while processing the command. Original error: el2.dispatchEvent is not a function. (In 'el2.dispatchEvent(new MouseEvent("mousedown",{button:0}))', 'el2.dispatchEvent' is undefined) when running "execute/sync" with method "POST"

Maybe I'm not doing it right.

As for the architecture, I agree that it's not ideal to trigger internal DOM events. The only possible cause that comes to mind is that maybe the event handlers in ToolbarButton.tsx are too selective for webdriver:

      onMouseDown={isTouch ? undefined : tapDown}
      onClick={isTouch ? undefined : tapUp}
      onTouchStart={isTouch ? tapDown : undefined}
      onTouchEnd={isTouch ? tapUp : undefined}

[raineorshine]

Bypassing webdriver is not a good option unfortunately. Webdriver controls a full browser instance, so we should be able to do pretty much anything the user can do. Something else must be preventing this from working as expected.

[ethan-james]

The only possible cause that comes to mind is that maybe the event handlers in ToolbarButton.tsx are too selective for webdriver:

      onMouseDown={isTouch ? undefined : tapDown}
      onClick={isTouch ? undefined : tapUp}
      onTouchStart={isTouch ? tapDown : undefined}
      onTouchEnd={isTouch ? tapUp : undefined}

I tried updating ToolbarButton to call tapUp onClick to see if that would be better for webdriver, but it didn't appear to have any effect.

[ethan-james]

@karunkop managed to get the tests passing in a branch, #4249, thanks for that! However, it was accomplished by making tapUp run onClick in addition to onTouchEnd, a change that only affects the iOS tests. On an actual device, onTouchStart calls preventDefault in order to prevent an active editable from blurring, which also cancels onClick. It worked in the tests because webdriverio doesn't call touch events at all, so the preventDefault never occurred.

I created another test branch, #4250, to continue testing. I added logging statements to tapDown and tapUp, and verified that they aren't called at all. I switched to a passing test that uses the tap helper, and added logging statements onTouchStart and onTouchEnd. Those also didn't get called, and the tap helper only works there because the behavior under test runs on focus or click.

Any chance that we could revisit these tests at a later time? We could be wrong about something here, but it doesn't seem like webdriverio has functional support for touch events. Possibly this is only the case for web apps. I found webdriverio/webdriverio#7997, which seems to be a related discussion, but not our exact situation.

[raineorshine]

Sure. If webdriverio has issues with touchevents, that warrants deeper research outside of this PR to figure out how to get proper mobile coverage. Touch events are critical for a lot of functionality.

[BayuAri]

Fixed of issues:

• Applying Text Color onto the whole thought text is not responded when a partial of the text already has the same text color #3904
• Undoing text background highlight makes the font invisible when it has a partial text with color #3905
• Issue C: Color is reset to default when trying to apply the same Font Color onto text with Partial Font Color
• Issue D: Color is reset to default when trying to apply the same Font Color onto Note with Partial Font Color
• Issue E: [iOS] Text Color is reset to default when apply any Font Color onto a text with Background Color
• Issue F: [iOS] Note Text Color is reset to default when apply any Font Color onto a text with Background Color
• Issue G: [Desktop] Undoing deletes the whole thought when applying the whole Background Color after applying a partial Background Color
• Issue H: [iOS] Undoing make the text invisible when applying the whole Background Color after applying a partial Background Color

Outstanding issue:

• Note's font color becomes invisible after undoing a background highlight selection #3903

[ethan-james]

Outstanding issue:

• Note's font color becomes invisible after undoing a background highlight selection #3903

🤦 I got so caught up with the iOS tests that I forgot to finish #3903. Sorry about that @BayuAri, it should be fixed now.

[BayuAri]

@ethan-james
Outstanding issue is fixed.
This also fixes #4274 and #4273

List of test cases
Fixed of issues:

• Applying Text Color onto the whole thought text is not responded when a partial of the text already has the same text color #3904
• Undoing text background highlight makes the font invisible when it has a partial text with color #3905
• Issue C: Color is reset to default when trying to apply the same Font Color onto text with Partial Font Color
• Issue D: Color is reset to default when trying to apply the same Font Color onto Note with Partial Font Color
• Issue E: [iOS] Text Color is reset to default when apply any Font Color onto a text with Background Color
• Issue F: [iOS] Note Text Color is reset to default when apply any Font Color onto a text with Background Color
• Issue G: [Desktop] Undoing deletes the whole thought when applying the whole Background Color after applying a partial Background Color
• Issue H: [iOS] Undoing make the text invisible when applying the whole Background Color after applying a partial Background Color
• Note's font color becomes invisible after undoing a background highlight selection #3903

[raineorshine]

Thank you, nice work