-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
fix(editor): Fix execution list item selection #5606
fix(editor): Fix execution list item selection #5606
Conversation
@@ -229,7 +224,7 @@ | |||
</div> | |||
<div v-else :class="$style.loadedAll">{{ $locale.baseText('executionsList.loadedAll') }}</div> | |||
</div> | |||
<div v-if="checkAll === true || isIndeterminate === true" :class="$style.selectionOptions"> | |||
<div v-if="numSelected > 0" :class="$style.selectionOptions"> | |||
<span> | |||
{{ $locale.baseText('executionsList.selected', { interpolate: { numSelected } }) }} |
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.
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.
Done by Csaba
@change="handleCheckAllChange" | ||
label="" | ||
/> | ||
<el-checkbox :value="checkAll" @change="handleCheckAllChange" label="" /> |
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.
this seems like an intentional feature we are losing here.. can you make sure to double check with Product and design first?
…single-execution-in-execution
…single-execution-in-execution
@@ -36,14 +36,26 @@ | |||
</el-checkbox> | |||
</div> | |||
|
|||
<el-checkbox |
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.
can you please add tests for this new feature
@@ -285,7 +301,8 @@ export default mixins(externalHooks, genericHelpers, executionHelpers, restApi, | |||
finishedExecutionsCount: 0, | |||
finishedExecutionsCountEstimated: false, | |||
|
|||
checkAll: false, | |||
allVisibleSelected: false, |
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.
this is the problem that I shared.. we should not have a flag for this.. it should be a set of ids instead..
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 added it because when all existing is selected there is a filter prop (deleteBefore
) created to tell the server which should be deleted.
In this case there are no IDs added to theselectedItems
(however we could do that in some cases that could be too much)
Great PR! Please pay attention to the following items before merging: Files matching
Files matching
Files matching
Files matching
Make sure to check off this list before asking for review. |
…single-execution-in-execution
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #5606 +/- ##
==========================================
+ Coverage 13.05% 13.15% +0.09%
==========================================
Files 2462 2462
Lines 112686 112690 +4
Branches 17498 17516 +18
==========================================
+ Hits 14715 14825 +110
+ Misses 97471 97366 -105
+ Partials 500 499 -1
... and 9 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
…single-execution-in-execution # Conflicts: # packages/editor-ui/package.json # pnpm-lock.yaml
…al of failed executions
…single-execution-in-execution
Got released with |
Got released with |
* fix(editor): Fix execution list item selection * fix(editor): Delete only selected executions * fix(editor): Fix clear selection * fix(editor): Fix clear selection * fix(editor): Fix clear selection * feat(editor): Add select all existing executions checkbox * fix(editor): Do not mark later loaded executions selected * test(editor): Add execution list unit test * fix(editor): Fix selection * test(editor): update execution selection test * fix(editor): Handle UI state when there is no execution * fix(editor): Remove unnecessary logic * test(editor): Add more execution list unit tests and fake data generation * test(editor): Add more execution list unit tests * test(editor): Simplifying test setup * chore: update pnpm lock after resolving merge conflocts * chore: fix package version * fix: Improved executions deletion to prevent crashing and fixed removal of failed executions * fix: Add comment to clarify why change was needed * fix: fix executions list bug when selecting all and changing filter * fix: fix execution lists running execution showing up on different workflow id * fix(editor): Deleting an execution while all are selected * fix(editor): Deleting an execution while all are selected --------- Co-authored-by: Omar Ajoue <[email protected]> Co-authored-by: Alex Grozav <[email protected]>
No description provided.