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

Merged
merged 4 commits into from
Nov 7, 2024
Merged

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}

nikku added 4 commits November 5, 2024 20:53
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 requested a review from philippfromme November 5, 2024 19:59
@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
Copy link
Contributor

@philippfromme philippfromme left a comment

Choose a reason for hiding this comment

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

Great finding and cleanup!

@nikku nikku merged commit 05d98ee into develop Nov 7, 2024
12 checks passed
@nikku nikku deleted the simplify-search-pad-preselect branch November 7, 2024 12:11
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Nov 7, 2024
@nikku
Copy link
Member Author

nikku commented Nov 7, 2024

Released with [email protected].

github-merge-queue bot pushed a commit to camunda/camunda that referenced this pull request Dec 10, 2024
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [bpmn-js](https://redirect.github.com/bpmn-io/bpmn-js) | [`17.11.1` ->
`18.1.1`](https://renovatebot.com/diffs/npm/bpmn-js/17.11.1/18.1.1) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/bpmn-js/18.1.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/bpmn-js/18.1.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/bpmn-js/17.11.1/18.1.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/bpmn-js/17.11.1/18.1.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

> [!WARNING]
> Some dependencies could not be looked up. Check the Dependency
Dashboard for more information.

---

### Release Notes

<details>
<summary>bpmn-io/bpmn-js (bpmn-js)</summary>

###
[`v18.1.1`](https://redirect.github.com/bpmn-io/bpmn-js/blob/HEAD/CHANGELOG.md#1811)

[Compare
Source](https://redirect.github.com/bpmn-io/bpmn-js/compare/v18.1.0...v18.1.1)

- `FIX`: adjust search to prioritize start of word and exact matches
([bpmn-io/diagram-js#953](https://redirect.github.com/bpmn-io/diagram-js/pull/953))
- `FIX`: ignore whitespace when searching
([bpmn-io/diagram-js#954](https://redirect.github.com/bpmn-io/diagram-js/pull/954))

###
[`v18.1.0`](https://redirect.github.com/bpmn-io/bpmn-js/blob/HEAD/CHANGELOG.md#1810)

[Compare
Source](https://redirect.github.com/bpmn-io/bpmn-js/compare/v18.0.0...v18.1.0)

- `FIX`: clear selection when opening search pad
([bpmn-io/diagram-js#947](https://redirect.github.com/bpmn-io/diagram-js/pull/947))
- `FIX`: correct dangling selection after search pad interaction
([bpmn-io/diagram-js#947](https://redirect.github.com/bpmn-io/diagram-js/pull/947))
-   `DEPS`: update to `[email protected]`

###
[`v18.0.0`](https://redirect.github.com/bpmn-io/bpmn-js/blob/HEAD/CHANGELOG.md#1800)

[Compare
Source](https://redirect.github.com/bpmn-io/bpmn-js/compare/v17.11.1...v18.0.0)

- `FEAT`: remove `outline` from `Viewer` modules
([#&#8203;2135](https://redirect.github.com/bpmn-io/bpmn-js/issues/2135))
- `FEAT`: make `Canvas` a focusable element
([bpmn-io/diagram-js#662](https://redirect.github.com/bpmn-io/diagram-js/pull/662))
- `FEAT`: implicit keyboard binding
([bpmn-io/diagram-js#662](https://redirect.github.com/bpmn-io/diagram-js/pull/662))
- `FEAT`: integrate with global `search`
([#&#8203;2235](https://redirect.github.com/bpmn-io/bpmn-js/pull/2235))
- `FEAT`: integrate `popup-menu` with `search`
([bpmn-io/diagram-js#932](https://redirect.github.com/bpmn-io/diagram-js/pull/932))
- `FEAT`: recognize modern `search` tokens in `search-pad`
([bpmn-io/diagram-js#932](https://redirect.github.com/bpmn-io/diagram-js/pull/932))
- `FIX`: correctly handle duplicate entries and whitespace in `search`
([bpmn-io/diagram-js#932](https://redirect.github.com/bpmn-io/diagram-js/pull/932))
- `FIX`: find `search` terms across all keys
([bpmn-io/diagram-js#932](https://redirect.github.com/bpmn-io/diagram-js/pull/932))
- `FIX`: `search` always returns tokens for matched items
([bpmn-io/diagram-js#932](https://redirect.github.com/bpmn-io/diagram-js/pull/932))
- `FIX`: prevent crash during label adjustment
([#&#8203;2239](https://redirect.github.com/bpmn-io/bpmn-js/issues/2239))
- `FIX`: keep existing loop characteristics when toggling through the
replace menu
([#&#8203;2251](https://redirect.github.com/bpmn-io/bpmn-js/pull/2251))
- `FIX`: prevent covering multi selection with black box in `Viewer`
([#&#8203;2135](https://redirect.github.com/bpmn-io/bpmn-js/issues/2135))
- `FIX`: generate types for main entry
([`986e2bb`](https://redirect.github.com/bpmn-io/bpmn-js/commit/986e2bb51ea301e6e0df56f3606a27424fb90179))
- `FIX`: correct handling of group name with whitespace only
([#&#8203;2231](https://redirect.github.com/bpmn-io/bpmn-js/issues/2231))
- `DEPS`: update to `bpmn-moddle@9`
([#&#8203;2114](https://redirect.github.com/bpmn-io/bpmn-js/pull/2114))
-   `DEPS`: update to `[email protected]`
-   `DEPS`: update to `[email protected]`

##### Breaking Changes

-   Require `Node >= 20`
- `Canvas` is now a focusable element and provides better support for
native browser behaviors. Focus can be controlled with new `focus` and
`restoreFocus` APIs.
- Keyboard is now implicitly bound to canvas SVG element. Calls to
`keyboard.bind` and `keyboard.bindTo` now result with a descriptive
console error and have no effect.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/camunda/camunda).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xOS4wIiwidXBkYXRlZEluVmVyIjoiMzkuMTkuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiYXV0b21lcmdlIl19-->
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.

3 participants