Multifurcating key and usability additions #4144
Open
Conversation
This file contains 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
Mostly in lead_spec.rb, from back before I had settled on a ruby style. I thought I would want to remove a bunch of the exhaustive position-checking specs, checking both actions on a left node and a right node, since I wrote those when I really didn't know why certain ways of adding nodes were failing and had no faith that the way that *was* working would continue working, but even now that I understand what was going on and have faith in the current code, I find myself choosing to keep them all anyway. I've already forgotten the trickiness of the issue causing the original consternation...
…ting internals This breaks other Leads tasks "Couplet" is now referred to as "Option set" in most places
…urcating internals
…ting keys; cleanup
…no children I initially wrote this to allow deleting a (mostly) arbitrary lead, even if it had children, but that seems a little risky, even with a big warning (in the sense that personally I think I would tend to say 'uh ... yeah, I think this is the subtree I want to delete (even though I haven't rechecked all of the nodes below this one)"). I'm open to doing the riskier operation though if others want to.
… facilitate reordering We could also just do a straight up 'move from i to j' operation, but the ui is slightly more involved and I guess I'm not expecting people to have many 5-10 sibling option sets, mostly 2,3,4 for which moving a lead one position at a time shouldn't be a big burden. But maybe I'm wrong.
The root of the inserted key is appended to the children of the lead inserted on. If your inserted key is for a genus and the key you're inserting into is for its parent family, the inserted genus root text will then change to be whatever condition at that point of the key leads to the conclusion that you're in that genus. (If you already had a lead with that text you'd want to copy it to the newly inserted root and then delete the old lead.)
…lowed bugs, other cleanup
I don't remember why I didn't do that in the first place...
I happened to check out this wikipedia article: https://en.wikipedia.org/wiki/Single-access_key which says "At each point in the decision process, multiple alternatives are offered, each leading to a result or a further choice. The alternatives are commonly called "leads", and the set of leads at a given point a "couplet". ... If the key has several choices it is described as polychotomous or polytomous. If the entire key consists of exactly two choices at each branching point, the key is called dichotomous." I actually prefer 'couplet' over 'option set' (I think, only because it's the more well-known/recognized term), in spite of the jarring reaction I get to seeing couplet refer to something with other than two parts.
…Show Lead The img is display: block since it's a flex item, so the margin-bottom was being applied to the img inside its border container, making the image overflow when the img was vertically large enough to fill the fixed vertical border container height (something like that).
…let' button I found the logic here to be pretty tricky.
globalId doesn't exist in the calling component and there's no globalId prop in CitationTopicForm - only found this from seeing a Vue warning in the console.
…or leads Previously I balked at letting users destroy an entire subtree, but I changed my mind. This delete is only available when children.size >= 2. If you want to delete a child when children.size == 2 you have to add a lead first and then delete the one you want. I've still chosen to make this different than destroy though, since /leads/1 --> Delete calls destroy (and destroy only) and there's really no way afaik to explicitly tell the user they'll be deleting an entire subtree in that case when they're not on a root node.
It would leave a lead with one child, which I've been unofficially avoiding (it's not entirely supported right now, I guess the thinking is that a completed key shouldn't have it, and we can support all needed edits while still requiring either 0 or >= 2 children).
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.
The main change is to make keys multifurcating instead of just dichotomous (with UI now using "Multifurcating" instead of "Dichotomous").
To support that, added two new actions:
Other changes:
When I started this work I thought couplet referred only to the case where there were exactly two options, so I changed a lot of wording to "option set" instead of "couplet" - but then I saw https://en.wikipedia.org/wiki/Single-access_key which implies the term 'couplet' can be applied to any number of leads, so I changed everything back to 'couplet'. I think I prefer that for clarity/common use (even though internally I still have a strong feeling that couplet should match with 2). Preference?