NUX: Prevent duplicate DotTips from appearing #8642
Conversation
When there are multiple instances of `DotTip` that reference the same tip identifier, only show the first instance that was mounted.
noisysocks
added
[Type] Bug
| @@ -13,6 +13,43 @@ export function triggerGuide( tipIds ) { | |||
| }; | |||
| } | |||
|
|
|||
| /** | |||
| * Returns an action object that, when dispathed, associates an instance of the | |||
There was a problem hiding this comment.
minor typo here - dispathed -> dispatched
| case 'REGISTER_TIP_INSTANCE': | ||
| return { | ||
| ...state, | ||
| [ action.tipId ]: union( state[ action.tipId ] || [], [ action.instanceId ] ), |
There was a problem hiding this comment.
Found this quite a hard line to read initially. I wonder if this would be more succinct? I think they do the same thing.
uniq( [ ...state[ action.tipId ], action.instanceId ] )
There was a problem hiding this comment.
We need to handle the case where state[ action.tipId ] is undefined.
uniq( [ ...state[ action.tipId ] || [], action.instanceId ] )Maybe I'll just use a temporary variable.
const existingInstanceIds = state[ action.tipId ] || [];
uniq( [ ...existingInstanceIds, action.instanceId ] )| return false; | ||
| } | ||
|
|
||
| if ( instanceId ) { | ||
| const [ firstInstanceId ] = state.tipInstanceIds[ tipId ] || []; |
There was a problem hiding this comment.
Was wondering - if we only use the first instance Id, do the others need to be stored in tipInstanceIds? Potentially the value of tipInstanceIds[ tipId ] could just be the instance Id of the first instance of that tip, instead of an array of all the tips. Maybe renamed to tipActiveInstanceId or something
There might be another use case that I'm missing, though.
There was a problem hiding this comment.
The problem is that it doesn't handle the case where the first/active instance is unmounted because of e.g. a block removal. When this happens, the tip should move to the next available instance. You actually encounter this in the reproduction steps for the bug above, because the first/active DotTip is unmounted when you insert the Columns block.
It's a pretty bizarre use-case, I'll admit. Hoping that we can make this all unnecessary either through #8050 or by otherwise iterating on these tips. Tammie and I have been working through some ideas on what to do with NUX...
|
We want to do 3.5 release today. I merged this PR. @noisysocks please open follow up and address the comments. At least the typo one 😄 |
This reverts commit 09dc71c.
Description
Fixes #8285.
When there are multiple instances of
DotTipthat reference the same tip identifier, only show the first instance that was mounted.I've done this by introducing the concept of "registration". When a
<DotTip id="foo">instance mounts, it registers itself as being associated with the "foo" tip. When it unmounts, the registration is removed. Only the first instance to have registered itself for any given tip will be made visible.How has this been tested?
Screenshots