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

Feature/optional params on components #475

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Feature/optional params on components #475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a0b9b5
make button tooltip optional
aswallace Mar 12, 2025
8b6be02
make modals not draggable by default
aswallace Mar 13, 2025
70d9bf4
update unit tests to use id instead of title
aswallace Mar 13, 2025
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
6 changes: 4 additions & 2 deletions packages/core/components/Buttons/BaseButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ interface Props {
menuItems?: IContextualMenuItem[];
onClick?: () => void;
text?: string;
title: string;
// title is only required if tooltip would be different from button text
// or if button does not have text (e.g., icon only)
title?: string;
}

/**
Expand Down Expand Up @@ -60,7 +62,7 @@ export default function BaseButton(props: Props) {
[styles.disabled]: props.disabled,
[styles.selected]: props.isSelected,
})}
data-testid={`base-button-${props.title}`}
data-testid={`base-button-${props.id}`}
ariaLabel={props.title}
disabled={props.disabled}
id={props.id}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/components/Buttons/PrimaryButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ interface Props {
menuItems?: IContextualMenuItem[];
onClick?: () => void;
text?: string;
title: string;
title?: string;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/core/components/Buttons/SecondaryButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ interface Props {
menuItems?: IContextualMenuItem[];
onClick?: () => void;
text?: string;
title: string;
title?: string;
}

/**
Expand Down
10 changes: 6 additions & 4 deletions packages/core/components/Buttons/test/BaseButton.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,21 @@ describe("<BaseButton />", () => {
it("does not perform click when disabled", () => {
// Arrange
const title = "Mock";
const testId = "mock-id";
let wasClicked = false;
const onClick = () => {
wasClicked = true;
};

const { getByTestId } = render(
<BaseButton disabled onClick={onClick} iconName="Download" title={title} />
<BaseButton disabled onClick={onClick} iconName="Download" title={title} id={testId} />
);

// (sanity-check) no click
expect(wasClicked).to.be.false;

// Act
fireEvent.click(getByTestId(`base-button-${title}`));
fireEvent.click(getByTestId(`base-button-${testId}`));

// Assert
expect(wasClicked).to.be.false;
Expand All @@ -30,20 +31,21 @@ describe("<BaseButton />", () => {
it("performs click when not disabled", () => {
// Arrange
const title = "Mock";
const testId = "mock-id";
let wasClicked = false;
const onClick = () => {
wasClicked = true;
};

const { getByTestId } = render(
<BaseButton onClick={onClick} iconName="Download" title={title} />
<BaseButton onClick={onClick} iconName="Download" title={title} id={testId} />
);

// (sanity-check) no click
expect(wasClicked).to.be.false;

// Act
fireEvent.click(getByTestId(`base-button-${title}`));
fireEvent.click(getByTestId(`base-button-${testId}`));

// Assert
expect(wasClicked).to.be.true;
Expand Down
3 changes: 1 addition & 2 deletions packages/core/components/DataSourcePrompt/FilePrompt.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,12 @@ export default function FilePrompt(props: Props) {
<form>
<label
aria-label="Browse for a file on your machine"
title="Browse for a file on your machine"
htmlFor="data-source-selector"
>
<SecondaryButton
iconName="DocumentSearch"
text="Choose file"
title="Choose file"
title="Browse for a file on your machine"
/>
</label>
<input
Expand Down
5 changes: 1 addition & 4 deletions packages/core/components/DataSourcePrompt/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,9 @@ export default function DataSourcePrompt(props: Props) {
</DefaultButton>
)}
<div className={styles.loadButtonContainer}>
{props.hideTitle && (
<SecondaryButton title="Cancel" text="CANCEL" onClick={() => onDismiss()} />
)}
{props.hideTitle && <SecondaryButton text="CANCEL" onClick={() => onDismiss()} />}
<PrimaryButton
disabled={!dataSource}
title="Load"
text="LOAD"
onClick={() => dataSource && onSubmit(dataSource, metadataSource)}
/>
Expand Down
1 change: 1 addition & 0 deletions packages/core/components/DateRangePicker/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export default function DateRangePicker(props: DateRangePickerProps) {
iconName="Clear"
onClick={onReset}
title="Reset"
id="reset-date"
/>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion packages/core/components/DateRangePicker/test/DateRangePicker.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe("<DateRangePicker />", () => {

// Hit reset
expect(onReset.called).to.equal(false);
fireEvent.click(getByTestId("base-button-Reset"));
fireEvent.click(getByTestId("base-button-reset-date"));
expect(onReset.called).to.equal(true);
});
});
4 changes: 2 additions & 2 deletions packages/core/components/FileDetails/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ export default function FileDetails(props: Props) {
disabled={isDownloadDisabled}
iconName="Download"
text="Download"
title="Download"
title="Download file to local system"
onClick={onDownload}
/>
</StackItem>
Expand All @@ -190,7 +190,7 @@ export default function FileDetails(props: Props) {
className={styles.primaryButton}
iconName="OpenInNewWindow"
text="Open file"
title="Open file"
title="Open file by selected method"
menuItems={openWithMenuItems}
/>
</StackItem>
Expand Down
5 changes: 3 additions & 2 deletions packages/core/components/Modal/BaseModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { TertiaryButton } from "../../Buttons";
interface BaseModalProps {
body: React.ReactNode;
footer?: React.ReactNode;
isDraggable?: boolean;
onDismiss?: () => void;
title?: string;
}
Expand All @@ -23,15 +24,15 @@ const DRAG_OPTIONS: IDragOptions = {
* for plugging content into.
*/
export default function BaseModal(props: BaseModalProps) {
const { body, footer, title, onDismiss } = props;
const { body, footer, isDraggable, title, onDismiss } = props;

const titleId = "base-modal-title";
return (
<Modal
isOpen
onDismiss={onDismiss}
containerClassName={styles.container}
dragOptions={DRAG_OPTIONS}
dragOptions={isDraggable ? DRAG_OPTIONS : undefined}
titleAriaId={titleId}
overlay={{ className: styles.overlay }}
>
Expand Down
2 changes: 1 addition & 1 deletion packages/core/components/Modal/CodeSnippet/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,5 +108,5 @@ export default function CodeSnippet({ onDismiss }: ModalProps) {
</>
);

return <BaseModal body={body} onDismiss={onDismiss} title="Code snippet" />;
return <BaseModal body={body} onDismiss={onDismiss} title="Code snippet" isDraggable />;
}
2 changes: 0 additions & 2 deletions packages/core/components/Modal/CopyFileManifest/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,14 +182,12 @@ export default function CopyFileManifest({ onDismiss }: ModalProps) {
className={styles.cancelButton}
onClick={onDismiss}
text="CANCEL"
title=""
/>
<PrimaryButton
className={styles.confirmButton}
disabled={loading || !!error}
onClick={onMove}
text="CONFIRM"
title=""
/>
</div>
}
Expand Down
1 change: 0 additions & 1 deletion packages/core/components/Modal/MetadataManifest/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ export default function MetadataManifest({ onDismiss }: ModalProps) {
iconName="Download"
onClick={onDownload}
text="DOWNLOAD"
title="Download"
/>
</div>
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/components/Modal/SmallScreenWarning/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ export default function SmallScreenWarning({ onDismiss }: ModalProps) {
<PrimaryButton
className={styles.okButton}
onClick={_onDismiss}
title="OK"
text="OK"
id="small-screen-ok"
/>
}
onDismiss={_onDismiss}
Expand Down
4 changes: 2 additions & 2 deletions packages/core/components/Modal/SmallScreenWarning/test/SmallScreenWarning.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe("<SmallScreenWarning />", () => {
fireEvent.click(checkbox);
await logicMiddleware.whenComplete();

const closeButton = await findByTestId("base-button-OK");
const closeButton = await findByTestId("base-button-small-screen-ok");
fireEvent.click(closeButton);
await logicMiddleware.whenComplete();

Expand Down Expand Up @@ -76,7 +76,7 @@ describe("<SmallScreenWarning />", () => {
fireEvent.click(checkbox); // uncheck
await logicMiddleware.whenComplete();

const closeButton = await findByTestId("base-button-OK");
const closeButton = await findByTestId("base-button-small-screen-ok");
fireEvent.click(closeButton);
await logicMiddleware.whenComplete();

Expand Down
1 change: 1 addition & 0 deletions packages/core/components/NumberRangePicker/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ export default function NumberRangePicker(props: NumberRangePickerProps) {
title="Reset filter"
onClick={onResetSearch}
iconName="Clear"
id="reset-filter"
/>
</div>
</div>
Expand Down
2 changes: 1 addition & 1 deletion packages/core/components/NumberRangePicker/test/NumberRangePicker.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe("<NumberRangePicker />", () => {

// Hit reset
expect(onSearch.called).to.equal(false);
fireEvent.click(getByTestId("base-button-Reset filter"));
fireEvent.click(getByTestId("base-button-reset-filter"));
expect(onSearch.called).to.equal(true);

// Should reset to min and max values
Expand Down
7 changes: 1 addition & 6 deletions packages/core/components/StatusMessage/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,7 @@ export default function StatusMessage() {
let cancelButton;
if (onCancel) {
cancelButton = (
<SecondaryButton
iconName=""
title="Cancel"
text="CANCEL"
onClick={onCancel}
/>
<SecondaryButton iconName="" text="CANCEL" onClick={onCancel} />
);
} else
onDismiss = () =>
Expand Down
2 changes: 2 additions & 0 deletions packages/core/components/TutorialTooltip/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export default function TutorialTooltip() {
iconName="CaretSolidLeft"
onClick={() => setTutorialStepIndex(previousStepIndex)}
title="Previous step"
id="tutorial-prev"
/>
<TertiaryButton
className={classNames(
Expand All @@ -90,6 +91,7 @@ export default function TutorialTooltip() {
iconName={tutorial.hasStep(nextStepIndex) ? "CaretSolidRight" : "Checkmark"}
onClick={selectNextTutorial}
title={tutorial.hasStep(nextStepIndex) ? "Next step" : "Finished"}
id="tutorial-next"
/>
</div>
</div>
Expand Down
6 changes: 3 additions & 3 deletions packages/core/components/TutorialTooltip/test/TutorialTooltip.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ describe("<TutorialTooltip />", () => {
expect(() => getByText(stepTwoMessage)).to.throw();

// Act
fireEvent.click(getByTestId("base-button-Next step"));
fireEvent.click(getByTestId("base-button-tutorial-next"));

// Assert
expect(getByText(stepTwoMessage)).to.exist;
expect(() => getByText(stepOneMessage)).to.throw();

// Act
fireEvent.click(getByTestId("base-button-Previous step"));
fireEvent.click(getByTestId("base-button-tutorial-prev"));

// Assert
expect(getByText(stepOneMessage)).to.exist;
Expand Down Expand Up @@ -86,7 +86,7 @@ describe("<TutorialTooltip />", () => {
expect(getByText(formattedTutorial)).to.exist;

// Act
fireEvent.click(getByTestId("base-button-Finished"));
fireEvent.click(getByTestId("base-button-tutorial-next"));

// Assert
expect(() => getByText(formattedTutorial)).to.throw();
Expand Down
2 changes: 1 addition & 1 deletion packages/web/src/components/DatasetDetails/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export default function DatasetDetails(props: DatasetDetailsProps) {
<PrimaryButton
className={styles.button}
iconName="Upload"
title="Load dataset"
title="Load dataset in the app"
text="LOAD DATASET"
onClick={() => props.onLoadDataset(datasetDetails)}
/>
Expand Down
4 changes: 2 additions & 2 deletions packages/web/src/components/DatasetDetails/test/DatasetDetails.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,11 @@ describe("<DatasetDetails />", () => {
);

// consistency checks, button exists & no actions fired
expect(getByLabelText("Load dataset")).to.exist;
expect(getByLabelText(/^Load dataset/)).to.exist;
expect(onLoadDataset.called).to.equal(false);

// Act
fireEvent.click(getByLabelText("Load dataset"));
fireEvent.click(getByLabelText(/^Load dataset/));

// Assert
expect(onLoadDataset.called).to.equal(true);
Expand Down
2 changes: 1 addition & 1 deletion packages/web/src/components/Header/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export default function Header() {
<Link to="app">
<SecondaryButton
className={styles.startButton}
title="Get started"
title="Get started in the app"
text="GET STARTED"
/>
</Link>
Expand Down
4 changes: 2 additions & 2 deletions packages/web/src/components/Home/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export default function Home() {
<PrimaryButton
className={styles.optionButton}
iconName="Checkmark"
title="Get started"
title="Get started with your own data"
text="GET STARTED"
/>
</Link>
Expand All @@ -41,7 +41,7 @@ export default function Home() {
<PrimaryButton
className={styles.optionButton}
iconName="List"
title="View datasets"
title="Go to our datasets page"
text="VIEW DATASETS"
/>
</Link>
Expand Down