-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Activity state display for plans in Gantt and Time list views #7370
Conversation
…ties are selected
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #7370 +/- ##
==========================================
- Coverage 55.95% 55.91% -0.05%
==========================================
Files 662 666 +4
Lines 26432 26513 +81
Branches 2574 2585 +11
==========================================
+ Hits 14790 14824 +34
- Misses 10928 10974 +46
- Partials 714 715 +1
*This pull request uses carry forward flags. Click here to find out more.
... and 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Current Playwright Test Results Summary✅ 123 Passing - ❌ 2 Failing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 01/19/2024 11:09:50pm UTC) ❌ Failures📄 functional/plugins/flexibleLayout/flexibleLayout.e2e.spec.js • 1 FailureTest Case Results
📄 functional/clearDataAction.e2e.spec.js • 1 FailureTest Case Results
|
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Restricted Notebook with a page locked and with an embed @addinit Allows embeds to be deleted if page unlocked @addinit
Retry 1 • Initial Attempt |
12.50% (1)1 / 8 runfailed over last 7 days |
62.50% (5)5 / 8 runsflaked over last 7 days |
📄 functional/plugins/displayLayout/displayLayout.e2e.spec.js • 1 Flake
Test Case Results
Test Case | Last 7 days Failures | Last 7 days Flakes |
---|---|---|
Display Layout Toolbar Actions @localStorage can add/remove Image to a single layout
Retry 1 • Initial Attempt |
0% (0)0 / 8 runsfailed over last 7 days |
12.50% (1)1 / 8 runflaked over last 7 days |
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.
- Activity state should be settable from Gantt, and both compact and expanded Time List views.
- We need a
s-selected
property to be applied to Activities in the Time List views in the same way that it is in the Gantt view. - The Set Activity State dropdown should use '- Set Status -' as its label, not '- Select Activity State -'.
- When the state for an Activity hasn't been set, the Set Activity State dropdown needs
selected
to be applied to the '- Set Status -' option so it doesn't appear as a blank, like this:
- The current activity status needs to be displayed in the Inspector as 'Status' per the Figma mockup as on https://trunk.arc.nasa.gov/confluence/display/VIPERMS/Situational+Awareness+Needs+and+Design
- Activities in Compact view should have a visual style when selected.
- Same for Activities in Expanded view. Done in branch
timelist-compact-view
.
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.
Halfway done reviewing. Just a few minor comments right now, but I have more to comment on with the PlanActivitiesInspectorView and the way we're passing props there. It's late though and I'll need time to collect my thoughts, but we really ought to restructure this.
I'm seeing this problematic pattern where we pass lists of objects into components that shouldn't maintain their own state, instead of passing props individually so Vue can pick and choose which DOM elements to render. This is a huge performance concern, and it's the very same pattern that we spent a lot of time refactoring out of PlotSeries. I left comments on the other PR regarding similar structural issues.
src/plugins/plan/inspector/components/PlanActivityPropertiesView.vue
Outdated
Show resolved
Hide resolved
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.
super tiny change and then we're g2g
…ivity-state-display
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.
LGTM
…y-state-display
Closes #7369
Also see VIPEROMCT PR 205
Describe your changes:
Adds an inspector view to display a list of possible states for activities
Adds persistence of activity states
Adds selection of activity rows in time list and display of activity properties (like in the gantt view)
Testing notes:
The plan Json requires each activity to have an "id" in order to set it's activity state
By default filtering includes the 'name' and any information in a "properties" object for each activity. For example:
All Submissions:
Author Checklist
type:
label? Note: this is not necessarily the same as the original issue.Reviewer Checklist