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

DT-1219: Allow un-selecting auth domains on snapshot create #1753

Merged
merged 3 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/components/common/FullViewSnapshotButton.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ describe('FullViewSnapshotButton', () => {

it('allows selecting an auth domain', () => {
cy.get('button').click();
cy.get('#authorization-domain-select').parent().click();
cy.get('#select-authorization-domain-select').parent().click();
cy.get('[data-cy=menuItem-group2]').click();
cy.get('#authorization-domain-select').should('have.value', 'group2');
cy.get('#select-authorization-domain-select').should('have.value', 'group2');
});

it('allows changing the name and description', () => {
Expand Down
5 changes: 3 additions & 2 deletions src/components/common/FullViewSnapshotButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ function FullViewSnapshotButton({
<div style={{ marginTop: 8 }}>
<FormLabel
sx={{ fontWeight: 600, color: 'black' }}
htmlFor="authorization-domain-select"
htmlFor="select-authorization-domain-select"
>
Authorization Domain
<span style={{ fontWeight: 400, color: 'black' }}> - (optional)</span>
Expand All @@ -191,9 +191,10 @@ function FullViewSnapshotButton({
sx={{ height: '2.5rem' }}
disabled={userGroups ? userGroups.length <= 1 : true}
options={userGroups ? userGroups.map((group) => group.groupName) : []}
name="authorization-domain"
name="Select Authorization Domain"
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing the name here will affect tests, since that generates the IDs by which we grab the component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, thanks!
I'm now formatting the id with lodash's "kebab case", so hopefully that will at least generate a more meaningful id.

onSelectedItem={(event) => setSelectedAuthDomain(event.target.value)}
value={selectedAuthDomain || ''}
includeNoneOption={true}
/>
<div>
Authorization Domains restrict data access to only specified individuals in a group and
Expand Down
20 changes: 17 additions & 3 deletions src/components/dataset/data/JadeDropdown.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import _ from 'lodash';
import { MenuItem, FormControl, Select, SelectChangeEvent } from '@mui/material';
import { MenuItem, FormControl, Select, SelectChangeEvent, Box } from '@mui/material';

type IProps<T> = {
disabled: boolean;
Expand All @@ -9,9 +9,18 @@ type IProps<T> = {
options: T[];
value: T;
sx?: React.CSSProperties;
includeNoneOption?: boolean;
};

function JadeDropdown({ disabled, name, onSelectedItem, options, value, sx }: IProps<string>) {
function JadeDropdown({
disabled,
name,
onSelectedItem,
options,
value,
sx,
includeNoneOption,
}: IProps<string>) {
return (
<form autoComplete="off">
<FormControl disabled={disabled} variant="outlined" fullWidth>
Expand All @@ -20,13 +29,18 @@ function JadeDropdown({ disabled, name, onSelectedItem, options, value, sx }: IP
onChange={(event) => onSelectedItem(event)}
inputProps={{
name,
id: `${name}-select`,
id: `${_.kebabCase(name)}-select`,
}}
displayEmpty
renderValue={(val) => (!val ? name : val)}
data-cy={_.camelCase(name)}
sx={sx}
>
{includeNoneOption && (
<MenuItem key="None" value="">
<Box sx={{ fontStyle: 'italic' }}>None</Box>
</MenuItem>
)}
{options.map((opt) => (
<MenuItem key={opt} value={opt} data-cy={`menuItem-${opt}`}>
{opt}
Expand Down