mirror of
https://github.com/elastic/kibana.git
synced 2025-04-24 17:59:23 -04:00
[Embeddable Rebuild] Fix stuck unsaved changes (#190165)
Closes https://github.com/elastic/kibana/issues/190066 ## Summary In https://github.com/elastic/kibana/pull/189128#discussion_r1700724700, I suggested adding a `skip` to the comparators subscription in order to prevent `unsavedChanges` from emitting twice - however, while I was on the right track with this suggestion, I didn't look closely enough at how this worked. Essentially, this is how it worked without the skip when creating a brand new React embeddable: 1. We initialize `unsavedChanges` by calling `runComparators` to get its initial value - since it is a new embeddable and therefore **does not have** unsaved state, it returns the entire state per the early return in `runComparators` 2. The comparator subjects all emit and, after the debounce, `unsavedChanges` gets set **again** by calling `runComparators` - similarly, it returns the entire state due to the early return. 3. Unsaved changes emits twice unnecessarily - once at initialization, once after a debounce 🔥 By adding the `skip` where I suggested, this is what happened: 1. We initialize `unsavedChanges` by calling `runComparators` to get its initial value - it returns the entire state due to the early return. 2. We **ignore** the first emit of every comparator due to the `skip` - this is good, because unsaved changes emits only once! 3. But, when we go to save the dashboard, the placement of the `skip` made it so that the subscription **did not fire** when `lastSavedState` emits - it is waiting for the comparators to emit again before firing! So, the unsaved changes remains unnecessarily 🔥 Essentially, we want to `skip` the first emit of the **whole** `comparator` + `lastSavedState` observable that results from the `pipe` - **not** just the comparators. Otherwise, we don't get to the `combineLatestWith` until **after** the comparators emit a second time, which means we don't respond when`lastSavedState` emits after a React embeddable panel is freshly added. By rearranging the order of operations in the `pipe`, we achieve the desired behaviour. To test the above, move the `skip` to **before** the `lastSavedState` emit. Add a React embeddable panel, and try to save - unsaved changes will be stuck. On a fresh dashboard, add a React embeddable panel again, **edit** that React embeddable to trigger a comparator emit, **then** save - unsaved changes is cleared! ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
This commit is contained in:
parent
ce1bd65010
commit
71b90a1af4
1 changed files with 0 additions and 2 deletions
|
@ -12,7 +12,6 @@ import {
|
|||
combineLatestWith,
|
||||
debounceTime,
|
||||
map,
|
||||
skip,
|
||||
Subscription,
|
||||
} from 'rxjs';
|
||||
import {
|
||||
|
@ -74,7 +73,6 @@ export const initializeUnsavedChanges = <RuntimeState extends {} = {}>(
|
|||
subscriptions.push(
|
||||
combineLatest(comparatorSubjects)
|
||||
.pipe(
|
||||
skip(1),
|
||||
debounceTime(COMPARATOR_SUBJECTS_DEBOUNCE),
|
||||
map((latestStates) =>
|
||||
comparatorKeys.reduce((acc, key, index) => {
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue