refactor(react-storage): Primary to composable ActionStart #5935

Samaritan1011001 · 2024-10-22T22:33:54Z

Description of changes

Refactors the usage and implementation of Primary button to ActionStart button while making it composable.

Exposes ActionStartProps for a composable custom ActionStart component
Uses ControlContext to supply the data/information needed for the composable component to display it.
Introduces a useActionStart hook that converts the controlContext into props that is fed into the ResolvedComposable ActionStart component.
Adds tests

Issue #, if available

Description of how you validated changes

Next example app manual tests

Checklist

Have read the Pull Request Guidelines
PR description included
yarn test passes and tests are updated/added
PR title and commit messages follow conventional commit syntax
If this change should result in a version bump, changeset added (This can be done after creating the PR.) This does not apply to changes made to docs, e2e, examples, or other private packages.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

changeset-bot · 2024-10-22T22:33:58Z

⚠️ No Changeset found

Latest commit: 1569c1a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

packages/react-storage/src/components/StorageBrowser/composables/ActionStart.tsx

packages/react-storage/src/components/StorageBrowser/controls/ActionStartControl.tsx

cshfang · 2024-10-23T16:49:19Z

packages/react-storage/src/components/StorageBrowser/controls/types.ts

+    actionStart?: {
+      onClick?: () => void;
+      label?: string;
+      disabled?: boolean;
+    };


@calebpollman could you verify this is at least sort of the right place for this? I know this is all still in flux so just want to know if this is amenable as a solution now or if it's way off base and we could do better even in the interim

packages/react-storage/src/components/StorageBrowser/controls/hooks/useActionStart.tsx

...eact-storage/src/components/StorageBrowser/views/LocationActionView/CreateFolderControls.tsx

...ages/react-storage/src/components/StorageBrowser/views/LocationActionView/UploadControls.tsx

packages/react-native/src/primitives/Label/utils.ts

packages/react-storage/src/components/StorageBrowser/composables/ActionStart.tsx

packages/react-storage/src/components/StorageBrowser/controls/types.ts

...ages/react-storage/src/components/StorageBrowser/views/LocationActionView/UploadControls.tsx

...eact-storage/src/components/StorageBrowser/views/LocationActionView/CreateFolderControls.tsx

packages/react-storage/src/components/StorageBrowser/controls/hooks/useActionStart.tsx

.../react-storage/src/components/StorageBrowser/controls/hooks/__tests__/useActionStart.spec.ts

cshfang

LGTM TY

...s/react-storage/src/components/StorageBrowser/controls/__tests__/ActionStartControl.spec.tsx

packages/react-storage/src/components/StorageBrowser/controls/hooks/useActionStart.tsx

packages/react-storage/src/components/StorageBrowser/controls/ActionStartControl.tsx

…rimary

ashwinkumar6 · 2024-10-30T17:47:21Z

...eact-storage/src/components/StorageBrowser/views/LocationActionView/CreateFolderControls.tsx

+    actionsConfig: {
+      type: 'SINGLE_ACTION',
+      isCancelable: true,
+    },


Suggested change

actionsConfig: {

type: 'SINGLE_ACTION',

isCancelable: true,

},

nit
actionsConfig is optional, can be removed if not required

ashwinkumar6 · 2024-10-30T17:48:20Z

.../react-storage/src/components/StorageBrowser/controls/hooks/__tests__/useActionStart.spec.ts

+    actionsConfig: {
+      isCancelable: true,
+      type: 'BATCH_ACTION',
+    },


Suggested change

actionsConfig: {

isCancelable: true,

type: 'BATCH_ACTION',

},

nit:
same as comment above

ashwinkumar6

Thanks lgtm !!
A few nit comments

ashwinkumar6 · 2024-10-30T17:51:46Z

...react-storage/src/components/StorageBrowser/views/LocationActionView/DeleteFilesControls.tsx

+      tableData,
+      isActionStartDisabled: disablePrimary,
+      actionStartLabel: 'Start',
+    },
    actionsConfig: { type: 'BATCH_ACTION', isCancelable: true },


Suggested change

actionsConfig: { type: 'BATCH_ACTION', isCancelable: true },

nit:
Same suggestion as above on removing actionsConfig if it's not required.
Note: not sure if it's used by the DataTableControl or other components

Yeah, I didn't add it so don't want to remove it as it might be used anywhere, same for the other similar comment.

ashwinkumar6 · 2024-10-30T17:54:03Z

...ages/react-storage/src/components/StorageBrowser/views/LocationActionView/UploadControls.tsx

+    actionsConfig: {
+      type: 'BATCH_ACTION',
+      isCancelable: true,
+    },


Suggested change

actionsConfig: {

type: 'BATCH_ACTION',

isCancelable: true,

},

same as comment above

…rimary

reesscot · 2024-10-30T21:06:38Z

packages/react-storage/src/components/StorageBrowser/composables/ActionStart.tsx

@@ -0,0 +1,24 @@
+import React from 'react';
+import { ButtonElement } from '../context/elements';


nit: we typically add an empty line between third party imports and local imports

Suggested change

import { ButtonElement } from '../context/elements';

import { ButtonElement } from '../context/elements';

(really need to just have an eslint rule for this)

reesscot · 2024-10-30T21:09:47Z

...s/react-storage/src/components/StorageBrowser/controls/__tests__/ActionStartControl.spec.tsx

+  });
+
+  afterAll(() => {
+    useActionStartSpy.mockRestore();


Is this necessary, aren't the tests already isolated?

Samaritan1011001 requested a review from a team as a code owner October 22, 2024 22:33

Samaritan1011001 had a problem deploying to ci October 22, 2024 22:44 — with GitHub Actions Failure

Samaritan1011001 temporarily deployed to ci October 22, 2024 22:44 — with GitHub Actions Inactive

Samaritan1011001 temporarily deployed to ci October 22, 2024 23:16 — with GitHub Actions Inactive

cshfang requested changes Oct 23, 2024

View reviewed changes

Samaritan1011001 requested a review from cshfang October 23, 2024 23:06

Samaritan1011001 temporarily deployed to ci October 23, 2024 23:14 — with GitHub Actions Inactive

cshfang reviewed Oct 23, 2024

View reviewed changes

Samaritan1011001 requested a review from cshfang October 24, 2024 00:24

Samaritan1011001 temporarily deployed to ci October 24, 2024 00:32 — with GitHub Actions Inactive

cshfang previously approved these changes Oct 24, 2024

View reviewed changes

...s/react-storage/src/components/StorageBrowser/controls/__tests__/ActionStartControl.spec.tsx Show resolved Hide resolved

ashwinkumar6 reviewed Oct 24, 2024

View reviewed changes

packages/react-storage/src/components/StorageBrowser/controls/hooks/useActionStart.tsx Outdated Show resolved Hide resolved

ashwinkumar6 reviewed Oct 24, 2024

View reviewed changes

packages/react-storage/src/components/StorageBrowser/controls/ActionStartControl.tsx Outdated Show resolved Hide resolved

Samaritan1011001 dismissed cshfang’s stale review via 247580b October 24, 2024 21:12

Samaritan1011001 temporarily deployed to ci October 24, 2024 21:24 — with GitHub Actions Inactive

cshfang previously approved these changes Oct 24, 2024

View reviewed changes

Samaritan1011001 added 2 commits October 29, 2024 15:05

chore: update primary usages and add tests

c1d8f59

chore: update Primary to ActionStart in comments

ce21880

Samaritan1011001 added 5 commits October 29, 2024 15:05

chore: nits and clean up of disabled logic

8592936

chore: nits and update controlscontext shape

eea22d2

chore: add tests and remove props nested type

c812a87

chore: add tests for the composable ActionStart

c010bfd

chore: remove unwanted comment

5f8c2cc

Samaritan1011001 dismissed cshfang’s stale review via 5f8c2cc October 29, 2024 22:29

Samaritan1011001 force-pushed the feat-sb/composability-primary branch from d0583b6 to 5f8c2cc Compare October 29, 2024 22:29

Samaritan1011001 added 3 commits October 29, 2024 15:31

Merge branch 'feat-storage-browser/main' into feat-sb/composability-p…

dd97eb0

…rimary

chore: update Primary component in new places it was used on main

895397b

Merge branch 'feat-storage-browser/main' into feat-sb/composability-p…

fd4b4e8

…rimary

Samaritan1011001 temporarily deployed to ci October 30, 2024 01:09 — with GitHub Actions Inactive

ashwinkumar6 reviewed Oct 30, 2024

View reviewed changes

ashwinkumar6 approved these changes Oct 30, 2024

View reviewed changes

Merge branch 'feat-storage-browser/main' into feat-sb/composability-p…

1569c1a

…rimary

Samaritan1011001 temporarily deployed to ci October 30, 2024 21:00 — with GitHub Actions Inactive

reesscot reviewed Oct 30, 2024

View reviewed changes

reesscot approved these changes Oct 30, 2024

View reviewed changes

cshfang approved these changes Oct 30, 2024

View reviewed changes

Samaritan1011001 merged commit f36904e into feat-storage-browser/main Oct 31, 2024
33 checks passed

Samaritan1011001 deleted the feat-sb/composability-primary branch October 31, 2024 00:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(react-storage): Primary to composable ActionStart #5935

refactor(react-storage): Primary to composable ActionStart #5935

Samaritan1011001 commented Oct 22, 2024

changeset-bot bot commented Oct 22, 2024 •

edited

Loading

cshfang Oct 23, 2024

cshfang left a comment

ashwinkumar6 Oct 30, 2024

ashwinkumar6 Oct 30, 2024

ashwinkumar6 Oct 30, 2024

ashwinkumar6 Oct 30, 2024

ashwinkumar6 left a comment •

edited

Loading

ashwinkumar6 Oct 30, 2024

Samaritan1011001 Oct 30, 2024

ashwinkumar6 Oct 30, 2024

reesscot Oct 30, 2024 •

edited

Loading

reesscot Oct 30, 2024

		@@ -0,0 +1,24 @@
		import React from 'react';
		import { ButtonElement } from '../context/elements';

refactor(react-storage): Primary to composable ActionStart #5935

refactor(react-storage): Primary to composable ActionStart #5935

Conversation

Samaritan1011001 commented Oct 22, 2024

Description of changes

Issue #, if available

Description of how you validated changes

Checklist

changeset-bot bot commented Oct 22, 2024 • edited Loading

⚠️ No Changeset found

Choose a reason for hiding this comment

cshfang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashwinkumar6 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reesscot Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

changeset-bot bot commented Oct 22, 2024 •

edited

Loading

ashwinkumar6 left a comment •

edited

Loading

reesscot Oct 30, 2024 •

edited

Loading