-
Notifications
You must be signed in to change notification settings - Fork 5
Rework Run Action Buttons into Action Framework #1524
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
base: 12-10-rework_codeviewer_implementation
Are you sure you want to change the base?
Rework Run Action Buttons into Action Framework #1524
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
8737a95 to
73c5435
Compare
73c5435 to
f5b8ed7
Compare
f5b8ed7 to
0fdda14
Compare
0fdda14 to
6ac7507
Compare
1dd8730 to
0e92bae
Compare
f1367b4 to
1c75c23
Compare
0e92bae to
19707ab
Compare
1c75c23 to
22fcc25
Compare
19707ab to
726344b
Compare
22fcc25 to
19bc578
Compare
726344b to
5b89d37
Compare
19bc578 to
f8982e3
Compare
5b89d37 to
5f2112e
Compare
f8982e3 to
442d3d8
Compare
5f2112e to
0d02f62
Compare
442d3d8 to
b5bc3a7
Compare
0d02f62 to
1714799
Compare
f0775a7 to
e56a33a
Compare
cd10655 to
d4c116a
Compare
9212073 to
e92bc36
Compare
d4c116a to
43689aa
Compare
| return { | ||
| actions, | ||
| isYamlFullscreen, | ||
| handleCloseYaml, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm against grouping actions into one hook. It is much better create multiple hooks or use useQuery and useMutation within specific components.
maxy-shpfy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm against grouping actions into one hook. It is much better create multiple hooks or use useQuery and useMutation within specific components.
e92bc36 to
8c926e9
Compare
43689aa to
1b8d745
Compare
8c926e9 to
bb051b9
Compare
1b8d745 to
0b78866
Compare
|
Hmmm. The idea was to split out the logic from the UI, and improve readability + testability. I'm open to ideas - let's discuss in the New Year |
0b78866 to
6759ff2
Compare
bb051b9 to
07f6759
Compare
6759ff2 to
17a86e0
Compare
176371b to
9d0f76f
Compare
17a86e0 to
77f593f
Compare
9d0f76f to
5fe4a59
Compare
77f593f to
57ea5aa
Compare
5fe4a59 to
c929346
Compare
57ea5aa to
f6b8aa2
Compare
c929346 to
a5ce09e
Compare
f6b8aa2 to
b98cf0e
Compare
a5ce09e to
47f036b
Compare
b98cf0e to
f0ad36d
Compare
661532b to
eba55a1
Compare
f0ad36d to
dd65777
Compare

Description
Action buttons in RunDetails follow the Action type style (rather than passing a straight ReactNode e.g. styled button). This ensures consistency of the interface, and makes it easy to implement new actions later.
This change requires some rework of the various RunDetails Buttons and the TaskImplementation. To formulate the actions list a new
useRunActionshook was added. Tests for these components have been, similarly, moved into a new consolidated test file for the hook.Related Issue and Pull requests
Type of Change
Checklist
Screenshots (if applicable)
Test Instructions
No change to app functionality.
Confirm that all buttons in RunActions look consistent and continue to function as expected.
Additional Comments