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.

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

Jump to bottom

chore(connection-form): clean up connection form styles; address modal jumping behavior #5836

Merged
merged 6 commits into from
May 24, 2024
Merged

chore(connection-form): clean up connection form styles; address modal jumping behavior #5836

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6a9d5c0
chore(components): allow to control accordion state
gribnoysup May 23, 2024
b610db5
chore(connection-form): separate form layout styles from the content;…
gribnoysup May 23, 2024
a24de46
chore(form): make sure that all advanced tabs are consistently using …
gribnoysup May 23, 2024
6c7fd78
chore(form): add scrollable spacing at the end of the form content
gribnoysup May 23, 2024
45bec71
chore(form): fix typo
gribnoysup May 23, 2024
b022c56
chore(form): padding not margin
gribnoysup May 23, 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
26 changes: 19 additions & 7 deletions packages/compass-components/src/components/accordion.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState } from 'react';
import React, { useCallback, useRef, useState } from 'react';
import { spacing } from '@leafygreen-ui/tokens';
import { css, cx } from '@leafygreen-ui/emotion';
import { palette } from '@leafygreen-ui/palette';
Expand Down Expand Up @@ -50,16 +50,30 @@ const buttonHintStyles = css({
interface AccordionProps extends React.HTMLProps<HTMLButtonElement> {
text: string | React.ReactNode;
hintText?: string;
open?: boolean;
setOpen?: (newValue: boolean) => void;
}
function Accordion({
text,
hintText,
open: _open,
setOpen: _setOpen,
...props
}: React.PropsWithChildren<AccordionProps>): React.ReactElement {
const darkMode = useDarkMode();
const [open, setOpen] = useState(false);
const regionId = useId('region-');
const labelId = useId('label-');
const [localOpen, setLocalOpen] = useState(_open ?? false);
const setOpenRef = useRef(_setOpen);
setOpenRef.current = _setOpen;
const onOpenChange = useCallback(() => {
setLocalOpen((prevValue) => {
const newValue = !prevValue;
setOpenRef.current?.(newValue);
return newValue;
});
}, []);
const regionId = useId();
Copy link
Contributor

Choose a reason for hiding this comment

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

if I understand correctly, the localOpen is just to avoid having 'open' as a callback dependency? is that worth it since the component anyway re-renders when open changes? am I missing something?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not really, localOpen is needed so that the accordion can be used without the need to control the state, this is the common controlled / uncontrolled pattern for React components. In leafygreen they usually use open, setOpen as the optional props to control the state, if you're writing a input in React for example, you similarly don't need to pass it value and html elements would still work as expected

Copy link
Contributor

Choose a reason for hiding this comment

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

ah I see, thanks for the explanation!

const labelId = useId();
const open = typeof _open !== 'undefined' ? _open : localOpen;
return (
<>
<button
Expand All @@ -72,9 +86,7 @@ function Accordion({
type="button"
aria-expanded={open ? 'true' : 'false'}
aria-controls={regionId}
onClick={() => {
setOpen((currentOpen) => !currentOpen);
}}
onClick={onOpenChange}
>
<span className={buttonIconContainerStyles}>
<Icon glyph={open ? 'ChevronDown' : 'ChevronRight'} />
Expand Down
9 changes: 8 additions & 1 deletion packages/connection-form/src/components/advanced-connection-options.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,23 @@ function AdvancedConnectionOptions({
errors,
updateConnectionFormField,
connectionOptions,
open,
setOpen,
}: {
errors: ConnectionFormError[];
disabled: boolean;
updateConnectionFormField: UpdateConnectionFormField;
connectionOptions: ConnectionOptions;
}): React.ReactElement {
} & Pick<
React.ComponentProps<typeof Accordion>,
'open' | 'setOpen'
>): React.ReactElement {
return (
<Accordion
data-testid="advanced-connection-options"
text="Advanced Connection Options"
open={open}
setOpen={setOpen}
>
<div className={connectionTabsContainer}>
{disabled && (
Expand Down
22 changes: 16 additions & 6 deletions packages/connection-form/src/components/advanced-options-tabs/advanced-options-tabs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ const tabsStyles = css({
marginTop: spacing[2],
});

const tabContentStyles = css({
// Remove margin from the last form element so that we can consistently apply
// the same padding for the whole connection form content
'& > :last-child': {
marginBottom: 0,
},
});

const tabWithErrorIndicatorStyles = css({
position: 'relative',
'&::after': {
Expand Down Expand Up @@ -131,12 +139,14 @@ function AdvancedOptionsTabs({
data-testid={`connection-${tabObject.id}-tab`}
data-has-error={showTabErrorIndicator}
>
<TabComponent
errors={errors}
connectionStringUrl={connectionStringUrl}
updateConnectionFormField={updateConnectionFormField}
connectionOptions={connectionOptions}
/>
<div className={tabContentStyles}>
<TabComponent
errors={errors}
connectionStringUrl={connectionStringUrl}
updateConnectionFormField={updateConnectionFormField}
connectionOptions={connectionOptions}
/>
</div>
</Tab>
);
})}
Expand Down
95 changes: 47 additions & 48 deletions packages/connection-form/src/components/advanced-options-tabs/advanced-tab/advanced-tab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import {
RadioBox,
RadioBoxGroup,
TextInput,
css,
spacing,
} from '@mongodb-js/compass-components';
import type ConnectionStringUrl from 'mongodb-connection-string-url';
import type { MongoClientOptions, ReadPreferenceMode } from 'mongodb';
Expand Down Expand Up @@ -48,10 +46,6 @@ export const readPreferences: ReadPreference[] = [
},
];

const containerStyles = css({
marginTop: spacing[3],
});

function AdvancedTab({
updateConnectionFormField,
connectionStringUrl,
Expand Down Expand Up @@ -96,45 +90,48 @@ function AdvancedTab({
);

return (
<div className={containerStyles}>
<>
{/* Read Preferences */}
<Label htmlFor="read-preferences">Read Preference</Label>
<RadioBoxGroup
onChange={({ target: { value } }: ChangeEvent<HTMLInputElement>) => {
handleFieldChanged(
'readPreference',
// Unset the read preference when default is selected.
value === defaultReadPreference ? undefined : value
);
}}
value={readPreference ?? defaultReadPreference}
data-testid="read-preferences"
id="read-preferences"
size="compact"
>
<RadioBox
id="default-preference-button"
data-testid="default-preference-button"
key="defaultReadPreference"
value={defaultReadPreference}
checked={!readPreference}
<FormFieldContainer>
<Label htmlFor="read-preferences">Read Preference</Label>
<RadioBoxGroup
onChange={({ target: { value } }: ChangeEvent<HTMLInputElement>) => {
handleFieldChanged(
'readPreference',
// Unset the read preference when default is selected.
value === defaultReadPreference ? undefined : value
);
}}
value={readPreference ?? defaultReadPreference}
data-testid="read-preferences"
id="read-preferences"
size="compact"
>
Default
</RadioBox>
{readPreferences.map(({ title, id }) => {
return (
<RadioBox
id={`${id}-preference-button`}
data-testid={`${id}-preference-button`}
checked={readPreference === id}
value={id}
key={id}
>
{title}
</RadioBox>
);
})}
</RadioBoxGroup>
<RadioBox
id="default-preference-button"
data-testid="default-preference-button"
key="defaultReadPreference"
value={defaultReadPreference}
checked={!readPreference}
>
Default
</RadioBox>
{readPreferences.map(({ title, id }) => {
return (
<RadioBox
id={`${id}-preference-button`}
data-testid={`${id}-preference-button`}
checked={readPreference === id}
value={id}
key={id}
>
{title}
</RadioBox>
);
})}
</RadioBoxGroup>
</FormFieldContainer>

{/* Replica Set */}
<FormFieldContainer>
<TextInput
Expand Down Expand Up @@ -169,11 +166,13 @@ function AdvancedTab({
}
/>
</FormFieldContainer>
<UrlOptions
connectionStringUrl={connectionStringUrl}
updateConnectionFormField={updateConnectionFormField}
/>
</div>
<FormFieldContainer>
<UrlOptions
connectionStringUrl={connectionStringUrl}
updateConnectionFormField={updateConnectionFormField}
/>
</FormFieldContainer>
</>
);
}

Expand Down
62 changes: 30 additions & 32 deletions ...ction-form/src/components/advanced-options-tabs/authentication-tab/authentication-tab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import {
Label,
RadioBox,
RadioBoxGroup,
spacing,
css,
FormFieldContainer,
} from '@mongodb-js/compass-components';
import type ConnectionStringUrl from 'mongodb-connection-string-url';
import type { AuthMechanism } from 'mongodb';
Expand Down Expand Up @@ -66,12 +66,8 @@ const options: TabOption[] = [
},
];

const containerStyles = css({
marginTop: spacing[3],
});

const contentStyles = css({
marginTop: spacing[3],
display: 'contents',
});

function getSelectedAuthTabForConnectionString(
Expand Down Expand Up @@ -141,31 +137,33 @@ function AuthenticationTab({
const AuthOptionContent = selectedAuthTab.component;

return (
<div className={containerStyles}>
<Label htmlFor="authentication-method-radio-box-group">
Authentication Method
</Label>
<RadioBoxGroup
id="authentication-method-radio-box-group"
data-testid="authentication-method-radio-box-group"
onChange={optionSelected}
value={selectedAuthTab.id}
size="compact"
>
{enabledAuthOptions.map(({ title, id }) => {
return (
<RadioBox
id={`connection-authentication-method-${id}-button`}
data-testid={`connection-authentication-method-${id}-button`}
checked={selectedAuthTab.id === id}
value={id}
key={id}
>
{title}
</RadioBox>
);
})}
</RadioBoxGroup>
<>
<FormFieldContainer>
<Label htmlFor="authentication-method-radio-box-group">
Authentication Method
</Label>
<RadioBoxGroup
id="authentication-method-radio-box-group"
data-testid="authentication-method-radio-box-group"
onChange={optionSelected}
value={selectedAuthTab.id}
size="compact"
>
{enabledAuthOptions.map(({ title, id }) => {
return (
<RadioBox
id={`connection-authentication-method-${id}-button`}
data-testid={`connection-authentication-method-${id}-button`}
checked={selectedAuthTab.id === id}
value={id}
key={id}
>
{title}
</RadioBox>
);
})}
</RadioBoxGroup>
</FormFieldContainer>
<div
className={contentStyles}
data-testid={`${selectedAuthTab.id}-tab-content`}
Expand All @@ -177,7 +175,7 @@ function AuthenticationTab({
connectionOptions={connectionOptions}
/>
</div>
</div>
</>
);
}

Expand Down
26 changes: 13 additions & 13 deletions packages/connection-form/src/components/advanced-options-tabs/csfle-tab/csfle-tab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ const options: KMSProviderMetadata[] = [
},
];

const containerStyles = css({
marginTop: spacing[3],
});

const accordionContainerStyles = css({
marginTop: spacing[3],
});
Expand Down Expand Up @@ -127,14 +123,18 @@ function CSFLETab({
);

return (
<div className={containerStyles}>
<Banner>
In-Use Encryption is an Enterprise/Atlas-only feature of MongoDB.&nbsp;
{/* TODO(COMPASS-5925): Use generic In-Use Encryption URL */}
<Link href="https://dochub.mongodb.org/core/rqe-encrypted-fields">
Learn More
</Link>
</Banner>
<>
<FormFieldContainer>
<Banner>
In-Use Encryption is an Enterprise/Atlas-only feature of
MongoDB.&nbsp;
{/* TODO(COMPASS-5925): Use generic In-Use Encryption URL */}
<Link href="https://dochub.mongodb.org/core/rqe-encrypted-fields">
Learn More
</Link>
</Banner>
</FormFieldContainer>

<FormFieldContainer>
<TextInput
onChange={({ target: { value } }: ChangeEvent<HTMLInputElement>) => {
Expand Down Expand Up @@ -244,7 +244,7 @@ function CSFLETab({
/>
</FormFieldContainer>
)}
</div>
</>
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/connection-form/src/components/advanced-options-tabs/general-tab/general-tab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function GeneralTab({
updateConnectionFormField: UpdateConnectionFormField;
}): React.ReactElement {
return (
<div>
<>
<FormFieldContainer>
<SchemeInput
errors={errors}
Expand All @@ -30,7 +30,7 @@ function GeneralTab({
connectionStringUrl={connectionStringUrl}
updateConnectionFormField={updateConnectionFormField}
/>
</div>
</>
);
}

Expand Down
Loading
Loading