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

areas appears selected in UI after rebind even though its selected state !== true #378

Closed
techfg opened this issue Feb 17, 2021 · 1 comment · Fixed by #383
Closed

areas appears selected in UI after rebind even though its selected state !== true #378

techfg opened this issue Feb 17, 2021 · 1 comment · Fixed by #383
Labels
Milestone

Comments

@techfg
Copy link
Collaborator

techfg commented Feb 17, 2021

Describe the bug
Per the docs, rebind should apply the options passed and re-render but it should not modify state. Currently, after calling rebind, a selected area remains visibly selected but the area itself no longer has a state of selected (e.g. get API does not include the key).

To Reproduce
Steps to reproduce the behavior:

  1. Go to jsFiddle
  2. Select any member of the beatles - the member is visually selected and Selected Keys reflects the members name
  3. Click on Rebind
  4. Click on Update Keys - This is required due to issue onConfigured not called after rebind #377

Expected behavior
The member should maintain its selected state and the Selected Keys should still include the members name.

Screenshots
N/A

Desktop (please complete the following information):
N/A

Smartphone (please complete the following information):
N/A

Additional context
N/A

@techfg techfg added the bug label Feb 17, 2021
@techfg techfg added this to the 1.5.4 milestone Feb 17, 2021
@techfg
Copy link
Collaborator Author

techfg commented Feb 21, 2021

The docs indicate that rebind will ...change areas that are already selected to be rendered with the new options.

Prior to commit 3de37f3 (line 723) this was true. However, in this commit, the behavior changed to not persist previously selected state and instead, completely rebuild Area Data items.

This change does appear intentional as the test case related to rebind also changed (Line 336). The test case itself incorporates singleSelect but there was no code modified/added to condition on singleSelect value so its difficult to know for certain if previous selected state should have been retained and just respected singleSelect somehow or if this test is just confirming that even when there are more than one areas configured with singleSelect === true they are selected after rebind.

Given the code changes in rebind method to call buildDataset and the fact that there is clearly no attempt to try to preserve state, the only logical conclusion was that the behavior was intentionally changed despite the fact that there was no major version update.

The net result of this commit is that rebind & unbind/init are nearly identical with the only exception being the "prep" work of creating the wrapper, etc. This difference is actually the issue identified in #379.

Trying to find a reliable and deterministic approach to maintaining selected state while still respecting the options passed gets complicated and could lead to different behavior in certain situations. For example:

  1. How should singleSelect be treated if true. If there are areas in config marked as selected, the any previous areas selected should be deselected and only the configured areas remain. If there are no areas configured as selected but more than one areas has a state of selected, which one remains selected?
  2. If an area is configured as staticState (or it was configured but now isn't) how to adjust state accordingly

Based on all of this, shifting this issue from treating it as selected state should still be selected to the area should no longer be drawn since its no longer selected after a rebind. In short, previous selected state is ignored and everything is based on current configuration passed. As mentioned, this minimizes the value of rebind but does stay consistent with the changes made in 3de37f3 by simply closing the gap/bug where the UI doesn't match internal selected state.

One additional note regarding behavior rebind - As written, it expects that the AREAS have not been modified in any way expect for the mapKey and href. It will ignore any other changes to the underlying HTML and in some cases (e.g. manual manipulation of coords) will cause unexpected results. This also means the order of the areas in the HTML should not change, no new areas should be added and no areas should be removed. If any of these changes are required, unbinding and re-initializing (e.g. $img.mapster('unbind').mapster({ ... })) is required.

@techfg techfg changed the title selected state cleared after rebind areas appears selected in UI after rebind even though its selected state !== true Feb 21, 2021
techfg added a commit that referenced this issue Feb 21, 2021
Resolves #83 
Resolves #377
Resolves #378
Resolves #380
Resolves #381
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant