added UI for request merging #2
Conversation
There was a problem hiding this comment.
There we go, I commented the code with the things that have to be changed.
Also, I just relicensed the project under the MIT license, so if you are still willing to contribute add your name into the new license file following this first "portions" format https://opensource.stackexchange.com/a/8624 and put that in this pr as well.
| <button type="button" class="btn btn-warning" onclick="addRequestToList(this)" id="selectToMergeButton" requestId="">Select to merge</button> | ||
| </span> | ||
| {% endif %} | ||
| <span class="ml-2" id="detailsSpan"> |
There was a problem hiding this comment.
having this button always visible is a bit excessive, I think there should be a button in the details popup to "start a merge" which adds the request to the merge list and then only show this button if there is at least one item in the merge list.
additionally, its a bit confusing that the button is still available after the request is selected. there should be some indicator that tells the user the request is selected, probably changing the button into a deselect button and maybe changing some color to make it extra clear?
There was a problem hiding this comment.
i added a checkbox on toolbar to show/hide all buttons related to merging
templates/home.html
Outdated
| if (selectedRequests.length > 1) { | ||
| let mergeButton = $("<button></button>") | ||
| .attr("type", "button") | ||
| .click({ index: i }, mergeSelectedTo) |
There was a problem hiding this comment.
there should be some confirmation popup before the merge goes through, giving a list of which requests will be removed with their votes transferred to which request
There was a problem hiding this comment.
added second modal for merge confirmation with listed all selected requests and the expected outcome (votes sum on target request)
templates/home.html
Outdated
| function mergeSelectedTo(event) { | ||
| let targetIndex = event.data.index; | ||
| let targetId = selectedRequests.splice(targetIndex, 1)[0]; | ||
| let payload = { targetId: targetId, sourceIds: selectedRequests } |
There was a problem hiding this comment.
| let payload = { targetId: targetId, sourceIds: selectedRequests } | |
| let payload = { id: targetId, ids_from: selectedRequests } | |
For the sake of consistency with earlier backend code. I'm gonna have to adjust the backend later to parse json (required in this case because arrays, as opposed to all other endpoints) and handle the array, but ill do that when everything else is done.
There was a problem hiding this comment.
changed payload to your suggestion
|
i added changes you requested, also added button on first modal to quiclky clear all selection and some tooltips on buttons |
2 participants
No description provided.