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

Display the settings modal for remote spaces and update the copy for … #3885

Merged

Conversation

gatzjames
Copy link
Contributor

@gatzjames gatzjames commented Aug 5, 2021

Enables the space settings for remote spaces.

The name input field is disabled and read-only.
There is also a helper tooltip next to the label that points you to the website if you want to rename the remote space.
Deleting the space removes any local collections/documents and changes that were not synced.
The copy of the confirmation dialog is also changed to try to explain that.

Demo:

ins-790

Concerns:

  • Is the tooltip copy helpful for users?
  • Does the link point to the correct place? Currently points to https://app.insomnia.rest/app/teams
  • Is the confirmation dialog copy clear enough for users?

Closes INS-790

@wdawson
Copy link
Contributor

wdawson commented Aug 5, 2021

This looks alright to me. But I wonder if the fact that the remote space will not be deleted remotely should be clearer. @erickrico any thoughts? And @falondarville , not sure if you want to be included on in-product copy stuff like this but tagging you for now.

Copy link
Contributor

@develohpanda develohpanda left a comment

Choose a reason for hiding this comment

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

Does the link point to the correct place? Currently points to app.insomnia.rest/app/teams

Yes I think so; @DMarby please confirm?

LGTM, will rely on @wdawson for copy feedback.

@DMarby
Copy link
Contributor

DMarby commented Aug 6, 2021

Yes I think so; @DMarby please confirm?

Looks good to me

Copy link
Contributor

@wdawson wdawson left a comment

Choose a reason for hiding this comment

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

Seems like we should improve the clarity of what happens to the remote space. Included a suggestion but feel free to change if we thnk there is a better way to put it.

packages/insomnia-app/app/ui/redux/modules/space.ts Outdated Show resolved Hide resolved
@vercel vercel bot temporarily deployed to Preview August 12, 2021 15:04 Inactive
Copy link
Contributor

@wdawson wdawson left a comment

Choose a reason for hiding this comment

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

⛵LGTM, thanks for the changes!

@gatzjames gatzjames enabled auto-merge (squash) August 12, 2021 19:53
@gatzjames gatzjames merged commit 9b7dbe5 into Kong:develop Aug 12, 2021
@gatzjames gatzjames deleted the ins-790-handle-rename-delete-for-remote-space branch August 2, 2022 12:34
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.

4 participants