-
Notifications
You must be signed in to change notification settings - Fork 2k
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
feat: introduce request group settings (name, description, and move/copy) #3350
feat: introduce request group settings (name, description, and move/copy) #3350
Conversation
Adds the ability to modify the description for a Request Group in a modal shown by hitting settings in a dropdown. Identical implementation to RequestSettings. In addition allows renaming the Request Group as well as moving/copying it to a different workspace. Ref: Kong#3306
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, and welcome to the project! We currently have a code-freeze in place because we're transitioning to typescript (1:1, should be fairly simple to update this PR), so I won't merge or test this just yet. 🤗
The words "Request Group" here feels wordy personally, I prefer "Folder", but whatever you think is best.
Let's go with folder! RequestGroup
is just the verbiage used in the code but at some point in the past the UI started to use Folder
.
This also adds the support of copying/moving workspaces as it is almost identical to the Request implementation.
That seems fine to me because we get it for free by deriving the implementation from RequestSettingsModal
. It would be worth checking there is nothing special happening in the "Move" dropdown item here. If so, let's bring that over and remove this entry.
I think opening the docs (in the request view/area) and expanding the folder/group is an obvious and solid approach. It would also have the benefit of being easily expanded upon, allowing in the future to have Folder Plugins as buttons on the far right side of where requests have headers/authorization or other group/folder features (ie: authorization, headers, etc). Anyways, just like to ensure that I'm on the same page here.
My comment here, I feel like this should be a different issue, but I'm down to address it if you'd like, I personally really want some of those future features as being able to add authorization headers for ALL requests in a group would be AWESOME!
Yep these things would be separate issues for sure. Having workspace/folder defaults have been requested in the past. It's not in our internal roadmap yet (AFAIK) but if you want to solutions in respective issues I can get feedback from our designer about those. 😄
packages/insomnia-app/app/ui/components/sidebar/sidebar-request-group-row.js
Outdated
Show resolved
Hide resolved
packages/insomnia-app/app/ui/components/sidebar/sidebar-request-group-row.js
Outdated
Show resolved
Hide resolved
packages/insomnia-app/app/ui/components/modals/request-group-settings-modal.js
Outdated
Show resolved
Hide resolved
packages/insomnia-app/app/ui/components/modals/request-group-settings-modal.js
Outdated
Show resolved
Hide resolved
…odal Co-authored-by: Opender Singh <[email protected]>
Co-authored-by: Opender Singh <[email protected]>
No longer need Request Group Move as it is present in Settings. In addition, fix the implementation in Settings to be exactly the same as the implementation present in move-request-group-modal which is duplicate then remove current rather than solely an update.
Awesome, Typescript sounds like a good choice for a project this big.
Done as of 9e642d1!
Removed and made necessary changes to have the same implementation as the modal that was used prior in 90a6aa5. Weirdly the previous implementation was a duplicate followed by a remove, not entirely sure how that's different than an update, but if it works, it works 😆
Good to hear, I'll have some up in the near future 🙂 |
That checks out! The |
…st-group-descriptions
65188fc
to
9cf9759
Compare
4bef4d9
to
ee54114
Compare
@develohpanda fixed merge conflicts and ported new file to Typescript, should be ready for review again 👍 |
@develohpanda any idea why the CI Windows Test is failing? Looks like a permission issue (line) seems unrelated, but I don't know how others are passing and this has failed twice. Previous instance failed due to a connection reset (line). 🤷♂️ |
@NickHackman ah, windows has been a bit buggy recently and failing on random issues like that. I'll kick it off again! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me too. Thanks, @NickHackman for the PR! ⛵ Also, hi! I'm the new PM on the Insomnia team 👋 |
Thanks for all your help @develohpanda, appreciate it and I'll for sure be contributing more in the future. @wdawson That's awesome! I have some other issues open that I'd love your opinion on 😉 (#3368, #3351) Looks like Windows is failing again due to connection reset, if you don't mind @develohpanda restarting the build again |
Just FYI @NickHackman waiting on one more eng review before merging 😄 almost good to go |
@dimitropoulos whoops, missed the |
no worries @NickHackman! :) Thanks for your contribution! |
Is this in the current release? |
Hey @youssefsharief, this will be a part of the next release (which should be out this month) |
@develohpanda thanks |
Adds the ability to modify the description for a Request Group in a modal shown by hitting settings in a dropdown. Identical implementation to RequestSettings. In addition allows renaming the Request Group as well as moving/copying it to a different workspace.
closes: #3306
Screenshots
Following pattern set by Requests as this goes after plugins.
The words "Request Group" here feels wordy personally, I prefer "Folder", but whatever you think is best. This also adds the support of copying/moving workspaces as it is almost identical to the Request implementation.
My comment here, I feel like this should be a different issue, but I'm down to address it if you'd like, I personally really want some of those future features as being able to add authorization headers for ALL requests in a group would be AWESOME!
I appreciate all feedback, thanks! 👍