palantir / blueprint

A React-based UI toolkit for the web
https://blueprintjs.com/
Apache License 2.0
20.51k stars 2.15k forks source link

Fix Overlay2 storing stale onClose callback #6874

Open evansjohnson opened 5 days ago

evansjohnson commented 5 days ago

Fixes https://github.com/palantir/blueprint/issues/6748

Checklist

Changes proposed in this pull request:

There are 3 changes in this PR, will split to separate PRs on request but figured they are closely related:

  1. Fix cleaning up event listeners - previously we'd always be trying to remove the current version of the event listener that will have never been set yet.
  2. Ensure we re-attach latest version of event listeners if for example onClose updates. We handle onClose updating for esc key and backdrop click because the latest callback versions are always passed to the element those are attached to, but did not do a similar update to the document listeners. A test case has been added for this behavior.
  3. Ensure we don't call a stale version of the overlayWillClose callback on component unmount: evanj/overlay-fix?expand=1#diff-50ceef9007

Reviewers should focus on:

Screenshot

n/a

changelog-app[bot] commented 5 days ago

Generate changelog in changelog-dir>`packages/core/changelog/@unreleased`</changelog-dir

What do the change types mean? - `feature`: A new feature of the service. - `improvement`: An incremental improvement in the functionality or operation of the service. - `fix`: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way. - `break`: Has the potential to break consumers of this service's API, inclusive of both Palantir services and external consumers of the service's API (e.g. customer-written software or integrations). - `deprecation`: Advertises the intention to remove service functionality without any change to the operation of the service itself. - `manualTask`: Requires the possibility of manual intervention (running a script, eyeballing configuration, performing database surgery, ...) at the time of upgrade for it to succeed. - `migration`: A fully automatic upgrade migration task with no engineer input required. _Note: only one type should be chosen._
How are new versions calculated? - ❗The `break` and `manual task` changelog types will result in a major release! - 🐛 The `fix` changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease. - ✨ All others will result in a minor version release.

Type

- [ ] Feature - [ ] Improvement - [ ] Fix - [ ] Break - [ ] Deprecation - [ ] Manual task - [ ] Migration

Description

Fix Overlay2 storing stale onClose callback **Check the box to generate changelog(s)** - [ ] Generate changelog entry
svc-palantir-github commented 5 days ago

this should keep order closer to what it was before

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github commented 5 days ago

format

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

svc-palantir-github commented 4 days ago

better name and co-locate

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.