prosemirror/prosemirror-search
1
0
Fork 0
Code Issues Pull requests Activity

Override default selection background for active match #1

Closed
ocavue wants to merge 2 commits from ocavue/selection-css into main
pull from: ocavue/selection-css
merge into: prosemirror:main
Conversation 3 Commits 2 Files changed 1 +4 -1
ocavue commented 2024-05-28 11:06:42 +02:00 (Migrated from github.com)
Copy link

When the editor is focused and .ProseMirror-active-search-match exists, the selection background color (usually in light blue) and the orange active decoration will overlap. This PR fixes the issue by only applying background color to ::selection.

A tiny issue is that, in Chrome, the <span> and ::selection has different height, which causes the orange .ProseMirror-active-search-match is slightly higher than the yellow .ProseMirror-search-match. I think this doesn't bother users. Just FYI.

Before

https://github.com/ProseMirror/prosemirror-search/assets/24715727/68614750-d7ad-4666-9d45-139f7ce78759

After

https://github.com/ProseMirror/prosemirror-search/assets/24715727/8a0cbc4e-e2b6-433a-86cd-838c7e38b1ec

When the editor is focused and `.ProseMirror-active-search-match` exists, the selection background color (usually in light blue) and the orange active decoration will overlap. This PR fixes the issue by only applying background color to `::selection`. A tiny issue is that, in Chrome, the `<span>` and `::selection` has different height, which causes the orange `.ProseMirror-active-search-match` is slightly higher than the yellow `.ProseMirror-search-match`. I think this doesn't bother users. Just FYI. **Before** https://github.com/ProseMirror/prosemirror-search/assets/24715727/68614750-d7ad-4666-9d45-139f7ce78759 **After** https://github.com/ProseMirror/prosemirror-search/assets/24715727/8a0cbc4e-e2b6-433a-86cd-838c7e38b1ec
marijnh commented 2024-05-28 18:05:02 +02:00 (Migrated from github.com)
Copy link

I don't think it is a good idea to change the color of the selection when it is on a search match. I'm not sure the overlapping backgrounds are an issue at all.

I don't think it is a good idea to change the color of the selection when it is on a search match. I'm not sure the overlapping backgrounds are an issue at all.
ocavue commented 2024-05-28 19:24:29 +02:00 (Migrated from github.com)
Copy link

The .ProseMirror-active-search-match class only appears when the text selection exactly matches it. This means the orange color will never be shown without mixing to users, which seems odd from a design perspective.

Additionally, I noticed that the background of the text selection and the span decoration have different heights, creating some padding, shown below:

image

These issues wouldn't arise if the text selection and .ProseMirror-active-search-match don't need to cover the same range. It seems that most rich text editors like Google Docs and MS Word, unlike code editors, don't change the selection while searching, so they don't have this color overlapping issue.

That said, I'm satisfied with the current API. It's flexible, allowing me to easily implement the styling on my end. It also makes sense that this library provides basic implementation and lets users do further customization. Feel free to close this PR.

The `.ProseMirror-active-search-match` class only appears when the text selection exactly matches it. This means the orange color will never be shown without mixing to users, which seems odd from a design perspective. Additionally, I noticed that the background of the text selection and the span decoration have different heights, creating some padding, shown below: <img width="262" alt="image" src="https://github.com/ProseMirror/prosemirror-search/assets/24715727/a6f4206e-9dc0-490b-8912-6ed0a1a8e1f8"> These issues wouldn't arise if the text selection and `.ProseMirror-active-search-match` don't need to cover the same range. It seems that most rich text editors like Google Docs and MS Word, unlike code editors, don't change the selection while searching, so they don't have this color overlapping issue. That said, I'm satisfied with the current API. It's flexible, allowing me to easily implement the styling on my end. It also makes sense that this library provides basic implementation and lets users do further customization. Feel free to close this PR.
marijnh commented 2024-05-28 21:33:07 +02:00 (Migrated from github.com)
Copy link

A common thing to do is to search from some kind of search dialog, without focus on the editor, in which case we need some way to show which match is the currently active one. When the editor is focused, the visible selection mostly fills this role.

But yeah, I'd like to avoid making these complicated or interfering with the selection color by default, so let's close this and let people do things like that in their own styling.

A common thing to do is to search from some kind of search dialog, without focus on the editor, in which case we need some way to show which match is the currently active one. When the editor is focused, the visible selection mostly fills this role. But yeah, I'd like to avoid making these complicated or interfering with the selection color by default, so let's close this and let people do things like that in their own styling.

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
prosemirror/prosemirror-search!1
No description provided.