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.

Sign up for GitHub

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

Jump to bottom

Handle renaming conflicts #195

Merged
merged 13 commits into from
Jan 8, 2025
Merged

Handle renaming conflicts #195

merged 13 commits into from
Jan 8, 2025

Conversation

fde31
Copy link
Member

@fde31 fde31 commented Jan 2, 2025

This PR prevents overwriting existing presets (both on instance as well as set level), sets and setviews by ensuring name conflicts are marked as an error.

Note I made this PR on top of the views work to get uniqueness validation for SetViews in there already as well as resusing the getUniqueName util that was already in place here.

closes #147

@fde31 fde31 requested a review from x37v January 2, 2025 13:46
@fde31 fde31 linked an issue Jan 2, 2025 that may be closed by this pull request
FR: Improved naming collision handling when saving sets and preset #147
Open
x37v
x37v requested changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@x37v x37v left a comment

Choose a reason for hiding this comment

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

this is nice but the feature missing is to be able to overwrite existing presets and set presets. you used to be able to do that via specifying the same name when saving but it would be nicer to just do that via the action menu like we do we sets.

@fde31 fde31 force-pushed the fde/unique_renaming branch from 0fc9807 to 0ea5d8c Compare January 8, 2025 11:34
@fde31 fde31 requested a review from x37v January 8, 2025 12:44
x37v
x37v reviewed Jan 8, 2025
View reviewed changes
Copy link
Contributor

@x37v x37v left a comment

Choose a reason for hiding this comment

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

looks great, final thing is i think we should probably trim the name the user types in.
i can save "foo" and then "foo "

fde31 added 10 commits January 8, 2025 16:33
@fde31
create unique name for new instance preset
dfc0573
@fde31
create unique names for new Sets as well as new SetPresets
584bbee
@fde31
presetitems: auto focus input when triggering renaming
7b76b5c
@fde31
focus renaming input for sets and presets when submitting form
1a57f68
@fde31
validate setview, set and preset names to be unique and show input fi…
a294bfe 
…eld error
@fde31
added missing validateUniqueName callback dep for SetView and SetView…
722cb13 
… item comps
@fde31
set and preset save actions: added optional arg to enable / disable u…
8699c23 
…niquename generation
@fde31
instance and set presets now have overwrite support
44da659
@fde31
improved distinction between createSet and saveSet (overwrite) in the…
9c58b55 
… code and ensured that the confirm dialogue is always shown upon overwrite
@fde31
presets and sets: trim names
f766045
@fde31 fde31 force-pushed the fde/unique_renaming branch from 28ab953 to f766045 Compare January 8, 2025 16:36
@fde31
Copy link
Member Author

fde31 commented Jan 8, 2025

ok I rebased on fde/views, fixed a function name typo and then ensured trimmed names and also the set name handling was not aligned with the rest. would appreciate another look and test with the runner in case I missed something in my local QA

@fde31 fde31 requested a review from x37v January 8, 2025 16:43
@fde31 fde31 merged commit 2250351 into fde/views Jan 8, 2025
1 check passed
@fde31 fde31 deleted the fde/unique_renaming branch January 8, 2025 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@x37v x37v x37v approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FR: Improved naming collision handling when saving sets and preset
2 participants
@fde31 @x37v