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

Simplify search pad preselect #947

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Conversation

nikku
Copy link
Member

@nikku nikku commented Nov 5, 2024

Proposed Changes

I traced a bug that caused the preselect marker class not being removed from an element. In the process I found that the whole marker logic we introduced via #913 is not necessary, after all: We use standard selection, and attach a djs-search-open marker to the diagram container; this is enough for us to handle "special selection" using plain CSS:

capture iCAZbM_optimized

_The change in action, testing against bpmn-js.

Checklist

To ensure you provided everything we need to look at your PR:

  • Brief textual description of the changes present
  • Visual demo attached
  • Steps to try out present, i.e. using the @bpmn-io/sr tool
  • Related issue linked via Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}

Turns out we don't need to build custom marker behavior
to have custom pre-select styling.

Once the search pad is opened there exist the djs-search-open marker
class on the diagram container that we can use for special styling.
This ensures that all visual clutter is gone once the search pad
is opened. Of course, on <Escape> the search restores.
@nikku nikku self-assigned this Nov 5, 2024
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Nov 5, 2024
@nikku nikku requested review from a team and misiekhardcore and removed request for a team November 5, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Review pending
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant