-
Notifications
You must be signed in to change notification settings - Fork 1
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
Simplify the Specifier component #333
base: master
Are you sure you want to change the base?
Conversation
Also took out an unnecessary TODO.
We have a separate box for that now.
|
@gaurav I'm not sure whether this is ready or not to be reviewed as is? You requested a review and it doesn't have draft status, but you're also mentioning an outstanding bug (implying that that needs to get fixed first?). Perhaps the outstanding bug can be moved to its own issue and then reviewing this PR can go ahead with the caveat of the issue pending to be fixed? Or does this rather need to get fixed here? |
Oops, my bad -- yes, I intend to fix the outstanding bug before sending it to you for review (I've removed the request to review now, and I'll re-invite you once it's ready for review). I think it's one of those bugs that indicate that there might be deeper issues here, so I'd like to try to fix it before finalizing this -- but if I see an opportunity to separate it into its own bug, I'll definitely do that! |
The Specifier component has some complex logic that allows users to modify the properties of a specifier in multiple ways, but this can lead to the specifier being overwritten just be opening it (#263). This PR sets about restructuring this component so that you can no longer modify the specifier just by opening it -- you have to explicitly modify something to trigger a modification action. (This is the minimum we need to close #263).
Now, we can't just remove all of this code, because the way in which Vue Store's data model works, we can't just plug the UI directly into the data object -- we have to call
$store.commit('setSpecifierProps', {specifier, props})
or$store.commit('replaceTUnitForPhylogenyNode', {...})
to commit the results. So instead we need a model like this:loadSpecifier()
.updateSpecifier()
.Additionally, this PR:
Apomorphy
as an option for the specifier dropdown (closes Remove "Apomorphy" from specifier type as we've now implementing it elsewhere #331).Closes #320: we don't include any special support for UUIDs, but we do now note that they can be used. They appear as collection IDs.
Example taxon:
Example specimen:
Example specimen with a UUID: