Merged
Conversation
pdurbin
approved these changes
Jul 25, 2022
Member
pdurbin
left a comment
There was a problem hiding this comment.
Seems to be a good fix. Note that when I deployed the branch the problem was still present until I hit Shift-Refesh.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it: Replacement for #8849 that shows the correct difference from develop.
Fixes issue. Also fixes a related issue that the popup was not showing on the Edit Template page (i.e. for the dataset description). That may have been broken longer.
Which issue(s) this PR closes:
Closes #8846
Special notes for your reviewer: Looks like the change in how dv_rebind... was called made it's no argument constructor for popoverHTML be the one that initialized the popup. The popup, with the correct title and list of tags is called from the dataset and dataverse pages as well so it looks like just removing the no arg call fixes things.
For the template page, it looks like there hasn't been any call to the popoverHTML function with parameters for a while (the thing that made the dataverse and dataset pages work after removing the line noted above). So - guessing this has been broken earlier.
Suggestions on how to test this:
See issue - 3 places: check the dataset/metadata/description popup and the one in the issue for the dataverse description, and the dataset description in the template edit page. and verify that you see the tags.
Key thing to check is whether the popup is still there after some ajax refresh of the page. (Spot checking by changing the template used for a new dataset (suggested by @scolapasta) suggests it does still work, but perhaps there are other situations where it won't).