Skip to content

feat(connection-form): show a banner on the connection form explaining why you can't edit the connection COMPASS-8264 #6344

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

Merged
merged 2 commits into from
Oct 10, 2024
Merged

feat(connection-form): show a banner on the connection form explaining why you can't edit the connection COMPASS-8264 #6344

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc47119
show a banner on the connection form explaining why you can't edit th…
lerouxb Oct 10, 2024
0eec49d
edge case
lerouxb Oct 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions packages/connection-form/src/components/connection-form.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,22 @@ describe('ConnectionForm Component', function () {
);
});

context('protectConnectionStrings', function () {
it('should not render the banner by default', function () {
renderForm();
expect(
screen.queryByTestId('protect-connection-strings-banner')
).to.be.null;
});

it('renders a banner if protectConnectionStrings === true', function () {
renderForm({
protectConnectionStrings: true,
});
expect(screen.getByTestId('protect-connection-strings-banner')).to.exist;
});
});

// TODO(COMPASS-7762)
context.skip('when preferences.showFavoriteActions === false', function () {
it('should not render the favorite button', function () {
Expand Down
15 changes: 13 additions & 2 deletions packages/connection-form/src/components/connection-form.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ const headingWithHiddenButtonStyles = css({
},
});

const disabledConnectedConnectionBannerStyles = css({
const bannerStyles = css({
marginTop: spacing[400],
paddingRight: 0,
});
Expand Down Expand Up @@ -500,7 +500,7 @@ function ConnectionForm({
{disableEditingConnectedConnection && onDisconnectClicked && (
<Banner
data-testid="disabled-connected-connection-banner"
className={disabledConnectedConnectionBannerStyles}
className={bannerStyles}
>
<div className={disabledConnectedConnectionContentStyles}>
<div>
Expand All @@ -521,6 +521,17 @@ function ConnectionForm({
</div>
</Banner>
)}
{protectConnectionStrings && (
Copy link
Collaborator

@gribnoysup gribnoysup Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I know it's a corner case, but would it make more sense to make this banner show up only when you can actually edit the connection string taking connection status into account? Otherwise it's getting pretty packed in there when editing a connected one:

image

Copy link
Contributor Author

@lerouxb lerouxb Oct 10, 2024

Choose a reason for hiding this comment

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

Aah well spotted. Fixed.

<Banner
data-testid="protect-connection-strings-banner"
className={bannerStyles}
>
Advanced Connection Options are hidden while the &quot;Protect
Connection String Secrets&quot; setting is enabled. Disable the
setting to configure Advanced Connection Options or edit your
connection string.
</Banner>
)}
<ConnectionStringInput
connectionString={connectionOptions.connectionString}
enableEditingConnectionString={enableEditingConnectionString}
Expand Down
Loading