Skip to content

Conversation

@nabalone
Copy link
Contributor

@nabalone nabalone commented Jan 20, 2026

This change is Reviewable

@nabalone
Copy link
Contributor Author

Feels dangerous enough to me that maybe it should target 6.4 instead? What do you think?

}

// Detector above the canvas so we can detect hover and clicks of playback controls
.bloom-videoMouseDetector {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know what to call this. "videoInteractionDetector"? "videoMouseEventDetector"? If we decide not to preserve hover detection (see below) then we could call it "videoClickDetector"

&.bloom-videoReplayIcon {
display: flex;
.bloom-videoContainer.playing {
.bloom-videoMouseDetector:hover {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like we decided not to do hover behavior in BloomPlayer (e.g. BloomBooks/bloom-player@c31f0fc) but kept it in BloomDesktop. Do we still want this?

.filter(
(_, x) =>
!x.classList.contains("bloom-videoControlContainer"),
!x.classList.contains("bloom-videoControlContainer") &&
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what's going on here, we have a "Not sure about keeping this" comment above, but now there's no point in keeping a bloom-videoControlContainer without a bloom-videoMouseDetector, it won't work

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.

2 participants