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

Fix: Allow empty name when duplicate request #6138

Merged
merged 3 commits into from Jul 18, 2023

Conversation

pilotpirxie
Copy link
Contributor

@pilotpirxie pilotpirxie commented Jul 11, 2023

changelog(Improvements): Empty strings are now allowed when duplicating a request.

When duplicating request, an empty prompt with a name can be accepted but it's not processed. Insomnia handles empty names and displays the entire URL. Not handling empty names looks there like a bad user experience.

This PR allows proceeding with an empty string when duplicating a request.

Screenshot 2023-07-11 at 23 08 19

Copy link
Contributor

@jackkav jackkav left a comment

Choose a reason for hiding this comment

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

I agree, however its debatable that we should allow empty request names as it could cause some confusion, but then we would need validation. Thanks.

@filfreire
Copy link
Member

@pilotpirxie @jackkav this will cause a few unhandled cases in terms of user experience, for example Rename which also uses prompt modal:

image

Right now we won't display anything:
image

@jackkav
Copy link
Contributor

jackkav commented Jul 14, 2023

closes #6035

@jackkav
Copy link
Contributor

jackkav commented Jul 14, 2023

We could add required to the prompt modal input as an alternative to allowinf empty, but I don't think its so bad to be empty naming things is sometimes too much trouble, perhaps an autofilled name and required field would be better than allowing empty. This PR is still okay though since empty names shouldn't cause any damage elsewhere, just confusion for our future selves.

@pilotpirxie
Copy link
Contributor Author

@filfreire
When renaming seems also fine:

Screen.Recording.2023-07-14.at.12.46.21.mov

@pilotpirxie
Copy link
Contributor Author

@jackkav so, can it be merged? Checks have passed but I don't have permission to merge

@jackkav jackkav added this pull request to the merge queue Jul 17, 2023
Merged via the queue into Kong:develop with commit ee36950 Jul 18, 2023
9 checks passed
@filfreire
Copy link
Member

@pilotpirxie you can claim a free tshirt for this PR contribution - find more at https://konghq.com/community/open-source-contribution

jackkav pushed a commit to jackkav/insomnia that referenced this pull request Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants