Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve search in huge diagrams #2233

Closed
2 tasks
nikku opened this issue Sep 12, 2024 · 7 comments · Fixed by #2240
Closed
2 tasks

Improve search in huge diagrams #2233

nikku opened this issue Sep 12, 2024 · 7 comments · Fixed by #2240
Assignees
Labels
enhancement New feature or request ux

Comments

@nikku
Copy link
Member

nikku commented Sep 12, 2024

Is your feature request related to a problem? Please describe.

As a follow-up to #2187 I reviewed the new search in a huge diagram. What I found is that search may not be as useful anymore due to two core changes:

  • We do not keep the existing viewport, but reset the viewport to 1 (standard zoom), as a result, on smaller screens and large diagrams the "search context" is gone
  • We do no longer aggressively highlight the search results (with blue background). This leads to results being hardly visible in crowded diagrams.

Old search

capture oczmuT_optimized

New search

capture sFEAwq_optimized

Describe the solution you'd like

  • Viewport is kept when searching, to accommodate for user intend (do I want to have a quick overview ("find usages across the diagram", or "do I want to jump to a particular element")
  • We re-introduce the background for search results, this can be done easily though CSS:
    .djs-search-open .djs-outline {
      fill: var(--element-selected-outline-stroke-color) !important;
      fill-opacity: .1;
    }

Describe alternatives you've considered

No real alternatives; I need to be able to understand the "context" in which a particular element is used, to learn if it is relevant for my intend.

Additional context

None.

@nikku nikku added enhancement New feature or request ux labels Sep 12, 2024
@philippfromme
Copy link
Contributor

By keeping the viewport you mean the zoom level?

@barmac
Copy link
Member

barmac commented Sep 12, 2024

Minor thing:
I'd expect it to show the fragment around the found string.

Before:
image

After:
image

Ideal:
image

@barmac
Copy link
Member

barmac commented Sep 12, 2024

One thing which is annoying but present in the old solution as well is that the position is reset when I hit SHIFT without any changes to the string.

Screen.Recording.2024-09-12.at.17.07.39.mov

@barmac
Copy link
Member

barmac commented Sep 12, 2024

For the background, let's make sure the contrast is sufficient.

@lmbateman
Copy link

Possible future enhancements: Indicate how many results there are, provide a control for users to click through them, and highlight results on the screen, as the browser search does. The first time I tried out the new search, I searched for "termin", saw a list of results in the dropdown, pressed Return, and was surprised that only one element was selected in the diagram. I see now that you can scroll through the dropdown, and the results in the diagram are selected in turn as you scroll, but making the control a bit more obvious might be helpful.

image

@barmac barmac added needs discussion Needs further discussion and removed needs discussion Needs further discussion labels Sep 16, 2024
@barmac
Copy link
Member

barmac commented Sep 16, 2024

Perhaps it makes sense to have a separate bug issue for the lacking contrast? WDYT @nikku?

@barmac barmac added the needs discussion Needs further discussion label Sep 16, 2024
@nikku
Copy link
Member Author

nikku commented Sep 16, 2024

Perhaps it makes sense to have a separate bug issue for the lacking contrast? WDYT @nikku?

Yes. I'm not sure which contrast you refer to, please create a separate issue.

@nikku nikku added the ready Ready to be worked on label Sep 22, 2024 — with bpmn-io-tasks
philippfromme added a commit to bpmn-io/diagram-js that referenced this issue Sep 23, 2024
philippfromme added a commit to bpmn-io/diagram-js that referenced this issue Sep 24, 2024
@barmac barmac removed the needs discussion Needs further discussion label Sep 24, 2024
@philippfromme philippfromme added the needs review Review pending label Sep 24, 2024 — with bpmn-io-tasks
@philippfromme philippfromme removed the ready Ready to be worked on label Sep 24, 2024
nikku pushed a commit to bpmn-io/diagram-js that referenced this issue Sep 25, 2024
nikku pushed a commit to bpmn-io/diagram-js that referenced this issue Sep 25, 2024
@nikku nikku added fixed upstream Requires integration of upstream change and removed needs review Review pending labels Sep 25, 2024
nikku added a commit that referenced this issue Sep 25, 2024
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Sep 25, 2024
@bpmn-io-tasks bpmn-io-tasks bot removed the fixed upstream Requires integration of upstream change label Sep 25, 2024
@barmac barmac closed this as completed in dca5dfb Oct 1, 2024
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ux
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants