Refactor completion widget

[CollinBeczak]

Implemented to allow multiple editors to be opened per task without having to click the "cancel editing" button. Buttons now also have a set width so that they can now be inline if the width of the widget is wide enough allowing for more customization option for a users widget layout.

This was also added to resolve: #920.
Users will now be able to open unsupported editors for cooperative and tag fix challenges in the edge case user workflow where that is desired.

[codecov]

Codecov Report

Attention: Patch coverage is 21.10092% with 86 lines in your changes missing coverage. Please review.

Project coverage is 23.38%. Comparing base (3b07eea) to head (a6bcba0).

Files	Patch %	Lines
...omponents/UserEditorSelector/UserEditorSelector.js	19.29%	38 Missing and 8 partials ⚠️
...CooperativeWorkControls/CooperativeWorkControls.js	0.00%	10 Missing ⚠️
...kControls/TaskCompletionStep/TaskCompletionStep.js	54.54%	10 Missing ⚠️
...skDetails/ActiveTaskControls/ActiveTaskControls.js	0.00%	7 Missing ⚠️
...omponents/ReviewTaskControls/ReviewTaskControls.js	0.00%	5 Missing ⚠️
src/components/TaskTags/TaskTags.js	0.00%	4 Missing ⚠️
...ponents/InspectTaskControls/InspectTaskControls.js	0.00%	3 Missing ⚠️
...dgets/TaskCompletionWidget/TaskCompletionWidget.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2388      +/-   ##
==========================================
- Coverage   23.48%   23.38%   -0.11%     
==========================================
  Files         651      649       -2     
  Lines       22583    22514      -69     
  Branches     6938     6902      -36     
==========================================
- Hits         5304     5264      -40     
+ Misses      14455    14425      -30     
- Partials     2824     2825       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jake-low]

Regarding the refactor: looks really good. Significantly reduces total lines of code which I find is a pretty good metric for whether a refactor improves maintainability. Good catch on the unnecessary conditionals in ActiveTaskControls.js (where two different branches did effectively the same thing).

Regarding the new UX: I tested this briefly and while I like most of it, I was confused/had questions about a couple specifics:

1. Has the notion of "locking" a task for editing been removed recently? I no longer see "Cancel editing" when using the legacy mode (i.e. without the embedded Rapid editor) and I no longer see an option to "Inspect Task" when browsing challenges and clicking a task on the map. I thought that both of these options were present in the past but now I'm doubting myself.

2. I find the double-dropdowns for the editor selection quite confusing. One behaves like a selector (clicking one of the listed options closes the drawer and the newly selected option is displayed on the drawer's handle). The other behaves like an action menu (clicking one of the options executes an action, e.g. opening the selected editor in a new tab). I'm not sure why a user would want both, and in my (admittedly brief) time playing around with the branch I found myself repeatedly using the wrong one and being surprised by the results of my clicks.

   I think a less confusing pattern would be to present the editor picker as a button (which executes the action) with an attached drawer handle that opens a selector which can change what action the button will perform. A great example of this is Github's merge button. I think using something similar to that might be a good idea here. The button text would read "Edit in JOSM" or "Edit in Rapid" etc (so the user knows exactly what action will be performed) and the arrow can open a selection menu for changing the action of the button.

[CollinBeczak]

1. Has the notion of "locking" a task for editing been removed recently? I no longer see "Cancel editing" when using the legacy mode (i.e. without the embedded Rapid editor) and I no longer see an option to "Inspect Task" when browsing challenges and clicking a task on the map. I thought that both of these options were present in the past but now I'm doubting myself.

Im pretty sure that when a logged in user opens a task when the link is formatted like https://maproulette.org/challenge/${challengeId}/task/${taskId}, the task is automatically locked. I had a discussion with @mvexel, and if i remember correctly, we concluded that the cancel editing button had no effect on the editors and only served to revert the widget state back to the editor selecting view. If a user ever wanted to open multiple editors, or believed that canceling editing had an effect other than changing the widgets state, that could lead to confusion.

Now to answer your inspect task question on the marker popups, i think you are right, it's not related to this pr, but i do think that adding back that functionality would be useful. Also, since a task is automatically locked when opening a task in the link format described above, we might want to reevaluate any instances of that format to see if the user should be directed to the inspect view of a task instead.

2. I find the double-dropdowns for the editor selection quite confusing. One behaves like a selector (clicking one of the listed options closes the drawer and the newly selected option is displayed on the drawer's handle). The other behaves like an action menu (clicking one of the options executes an action, e.g. opening the selected editor in a new tab). I'm not sure why a user would want both, and in my (admittedly brief) time playing around with the branch I found myself repeatedly using the wrong one and being surprised by the results of my clicks.
   I think a less confusing pattern would be to present the editor picker as a button (which executes the action) with an attached drawer handle that opens a selector which can change what action the button will perform. A great example of this is Github's merge button. I think using something similar to that might be a good idea here. The button text would read "Edit in JOSM" or "Edit in Rapid" etc (so the user knows exactly what action will be performed) and the arrow can open a selection menu for changing the action of the button.

The new dropdown was added to resolve this ticket: #920, It always shows all the editors and allows users to open editors they wouldn't usually be able to in a cooperative or tag fix challenge. I like the new button idea you are suggesting, if its something that we would want to move forwards with, do you have any opinions on how/if we should implement the above feature request.

[jake-low]

Thanks for the detailed explanations.

Re: "locking" a task, I had assumed that there was some functionality tied to this action on the backend to help prevent multiple users from working on the same task at the same time. I think Tasking Manager has something like this and I may have misunderstood the intent of the MR feature through that lens.

Re: the dropdowns, I need to read the feature request you linked and play around with that user flow to understand the situation better. I hadn't realized that the two dropdowns might sometimes show different lists of editors (I forgot that challenge authors can specify which editors should be used, and that tag fix challenges don't actually require use of any editor). I'll reply here if I have any ideas about how to support that with a single dropdown.

[mvexel]

Pardon me if I misremember or misrepresented originally -- there is definitely value in being able to actively unlock a task to abandon work on it. This is perhaps sort of similar to skipping, but it doesn't add a state change to the task and doesn't move on to the next one. @jake-low locking a task does prevent others from working on it afaik. It should, for sure.

Opening a task in multiple editors definitely has value and we should move forward with this but I agree the two dropdowns are too confusing. I like the idea of having a 'picker dropdown' as well.

[jake-low]

The design of the picker looks good to me at this point. I'm not 100% sure about the pattern of having both selection items and clickable links in the same dropdown, but the link icons help differentiate them (idea: a checkmark next to the currently selected editor would help make it more clear that the top section is a selection list). I also think it might be nice to wrap the buttons (both the UserEditorSelector and the other task completion buttons) in a flex container so that if there's extra space they can grow a bit bigger. Anyway, the current state of this PR is an improvement over what we have today and we can iterate more on it in the future.

I'm still not completely clear on whether this PR removes a user's ability to explicitly unlock a task they're working on - it seems to from my testing but that might be user error on my part.

[jschwarz2030]

Pardon me if I misremember or misrepresented originally -- there is definitely value in being able to actively unlock a task to abandon work on it. This is perhaps sort of similar to skipping, but it doesn't add a state change to the task and doesn't move on to the next one. @jake-low locking a task does prevent others from working on it afaik. It should, for sure.

Opening a task in multiple editors definitely has value and we should move forward with this but I agree the two dropdowns are too confusing. I like the idea of having a 'picker dropdown' as well.

I spoke with Collin about this concern and I don't really see a risk here. Locking isn't tied to this widget or to opening an editor, it's only tied to landing on the task inspection page. There should be no change.

[jake-low]

I just realized that there's another way to manually unlock a task, by clicking the lock icon next to the challenge name. I thought the only way was to click the "Cancel editing" button in the task completion widget. So I don't have any concerns about merging this.