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

feat(connect): Save & Connect button COMPASS-5776 #3163

Merged
merged 4 commits into from Jun 15, 2022
Merged

Conversation

lerouxb
Copy link
Contributor

@lerouxb lerouxb commented Jun 3, 2022

In the New Connection dialog (ie. when there's no existing favorite connection involved), display a Save & Connect button in addition to the existing Save and Connect buttons. This will pop up the Save connection to favorites dialog and replace the dialog's Save button with a Save & Connect button. Clicking that button saves the connection as a favorite and connects.

@github-actions github-actions bot added feat and removed feat labels Jun 3, 2022
@Anemy Anemy self-requested a review June 10, 2022 20:16
@@ -296,6 +310,10 @@ function ConnectForm({
...favoriteInfo,
},
});

if (saveConnectionModal === 'saveAndConnect') {
onSubmitForm();
Copy link
Member

Choose a reason for hiding this comment

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

I think the connection options in the form don't have the recently set favorite settings on the first connection.
After save and connect with a new connection:
(Lack of connection name)
Screen Shot 2022-06-14 at 1 45 02 PM
After reloading the favorites list and connecting to the favorite:
(has connection name)
Screen Shot 2022-06-14 at 1 45 07 PM

Not sure this is a blocker, but I could see it tripping up some folks.

Due to the nature of how the useConnectForm is setup it may take some extra work to have it update the current connection options and then connect from there. Maybe we could add an optional argument overload to the onSubmitForm function to explicitly connect to the passed options? Totally up to you implementation wise, just suggesting a thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is an existing problem. ie if you hit save via the favorite modal and then connect on main now it also happens.

Due to that I'm not sure what's best practise - make this PR larger and fix it now or fix it in a followup?

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it's the same, yes. When saving happens we don't update the current connection settings. As for best practice, it's up to you, since it won't change this code much I think it would be nice to have it in another pr. It could be a separate fix then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Co-authored-by: Rhys <Anemy@users.noreply.github.com>
@lerouxb lerouxb merged commit 8c372ae into main Jun 15, 2022
@lerouxb lerouxb deleted the save-and-connect branch June 15, 2022 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants