Skip to content
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

feature bulking: added edition modes [MRXNM-58] #1690

Merged
merged 4 commits into from
Sep 10, 2024

Conversation

andresgnlez
Copy link
Member

@andresgnlez andresgnlez commented Sep 4, 2024

Substitute this line for a meaningful title for your changes

Overview

This PR solves the casuistic of an user being able to perform unwanted changes when performing certain bulking actions.

image

Designs

Link to the related design prototypes (if applicable)

Testing instructions

Please explain how to test the PR: ID of a dataset, steps to reach the feature, etc.

Feature relevant tickets

Link to the related task manager tickets


Checklist before submitting

  • Meaningful commits and code rebased on develop.
  • If this PR adds feature that should be tested for regressions when
    deploying to staging/production, please add brief testing instructions
    to the deploy checklist (docs/deployment-checklist.md)
  • Update CHANGELOG file

Copy link

vercel bot commented Sep 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
marxan ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 10, 2024 3:05pm

@andresgnlez andresgnlez self-assigned this Sep 4, 2024
@andresgnlez andresgnlez added the Frontend Everything related frontend label Sep 4, 2024
@andresgnlez
Copy link
Member Author

@hotzevzl I think the solution (and the pic) are quite self-explanatory, but let me know if something comes up.

Copy link
Member

@hotzevzl hotzevzl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andresgnlez thank you - I like your solution, clear and simple (at least to understand and use, not sure about the implementation itself 😬 but that was on you, in any case 😅).

I think this solves nicely the UX limitations that users were reporting, from the original design.

One small thing - while trying to punch in some random values when using this feature branch, I run into a small glitch (probably unrelated, maybe it's been there all along and we never noticed nor did users): for specific values (so far, I've been able to trigger it by setting target to 55% specifically, I couldn't figure out other such values), I think that the value gets mangled before being sent to the API, setting it to 55.00000000000001 🤔

The screenshots below show what I see (this is Firefox 129.0 on Arch Linux, in case it's browser-related) after saving a bulk edit with target set by myself to 55, once I see the feature list again:

Screenshot 2024-09-06 at 09-44-53 Scenarios Edit test 1 - MRXN23-524 refetching cost surfaces test 1_x

And once I reopen the bulk edit modal (while selecting only one feature, of course - if I select more than one I get the default 50 and 1 values respectively):

Screenshot 2024-09-06 at 09-44-37 Scenarios Edit test 1 - MRXN23-524 refetching cost surfaces test 1_x

@hotzevzl
Copy link
Member

hotzevzl commented Sep 6, 2024

also in terms of UX here - maybe when bulk editing with > 1 feature selected, rather than showing some default values (50% and 1 respectively for target and fpf), should we leave the inputs blank? Really minor, and I don't think this will ever be a real cause of confusion for users (such as "whoops, I set all the targets to 50%, I didn't really want to do that, I just forgot to edit that value"), and it's also something that - if ever happens - is easy to recover from, by bulk editing again the relevant features, so I'm ok with us leaving things as they are (and have always been, IIRC) in terms of these defaults when the bulk edit modal is brought up.

@hotzevzl
Copy link
Member

hotzevzl commented Sep 6, 2024

and another small thing - at last in FF 129, if I bulk select all features by toggling the check box above the features, then edit the features and save the edits, once the processing finishes, I am back to the list of features, with none of the features selected, but the "select all" check box selected - again, super minor and I'd try to avoid cramming more things into this PR, but in case it's something really easy to change and regression-proof, please consider if it's worth changing:

Screenshot 2024-09-06 at 10-00-00 Scenarios Edit test 1 - MRXN23-524 refetching cost surfaces test 1_x

@andresgnlez andresgnlez merged commit e9389b9 into develop Sep 10, 2024
12 of 13 checks passed
@andresgnlez andresgnlez deleted the chore/feature-bulk-modes branch September 10, 2024 15:13
@hotzevzl hotzevzl changed the title feature bulking: added edition modes feature bulking: added edition modes [MRXNM-58] Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend Everything related frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants