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.

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

Advanced Timeline Filtering #1535

Open
wants to merge 92 commits into
base: develop
Choose a base branch
from

Conversation

AaronPlave
Copy link
Contributor

@AaronPlave AaronPlave commented Oct 30, 2024

This PR implements dynamic activity filtering for the timeline and updates resource and external event layer management. These changes aim to give users finer control over how activities in a plan are filtered and rendered on the timeline.

Tickets Addressed:

image

Changes:

  • Update the way activities are specified in the UI view to support dynamic filtering. This includes a list of manually selected types ORed with a list of dynamically specified types ANDed with a list of global filters on the directives and spans themselves. Each resulting type can also contain a list of global filters targeted at instances of that specific type.
  • Implement activity filtering popover. Popover provides the UI to back the view changes for activity filtering. The user can manually add and remove types, dynamically select types by Type and Subsystem, globally filter instances by Name, Parameter (filtered appropriately by the current list of resulting types), Tag, and Scheduling Goal ID. The user can also add and remove filters (the same as global filters) on specific resulting types. Popover component is draggable and resizable.
  • Update Timeline Items browser (Activity, Resource, Event Types panel) to conform to the new filtering schema and use dynamic filtering whenever appropriate.
  • Change the UI view resource filter specification to be a string instead of a list since only one resource can be logically selected for a layer.
  • Update resource filtering to be single select
  • Update resource layer editor to use icon segmented controls for line vs x-range plot
  • Ability to duplicate timeline layers
  • Update CssGrid with columnMinSizes and rowMinSizes props to support min width and height for css grid sections
  • Restyle searchable dropdown selected state appearance and add a checkmark
  • Add Draggable window component
  • Add Resizable box component
  • Fetch all initial/default activity type arguments so that Parameters can be filtered client side
  • Create a subsystem tags store derived from activity types store
  • Use Tanstack Virtual svelte package to virtualize the SearchableDropdown and TimelineItemList to achieve acceptable performance for long lists
  • Bump UI View version to 2 and create relevant migrations.

Testing

Activities

  1. Create a plan and open it
  2. The plan should have a row in the default view that shows all activities
  3. Open the activity row in the timeline editor
  4. Verify that in the "Activity Layers" section a layer exists with the name "All Activities"
  5. Click on the filter button containing "All Activities"
  6. A modal should appear
  7. Verify that the modal can be dragged around via the modal header and that it can be resized using the handle at the bottom right corner of the modal
  8. Type in a new name for the layer to the right of the modal title and verify that the activity layer in the editor reflects this name change
  9. Manually a type that exists in the plan from the first section in the modal and verify that only this type appears in the timeline (look at the instances count on the right side of the modal or look at the timeline)
  10. Add a dynamic type filter in the second section like "Type includes 'banana'" and verify that all types in the modal that contain "banana" are included in the resulting types list on the right of the modal and appear in the timeline
  11. Add a BiteBanana activity
  12. Add the global filter "parameter biteSize equals 1" and ensure that the BiteBanana activity shows up on the timeline
  13. Change the global filter to "parameter biteSize equals 2" and ensure that the BiteBanana activity does not show up on the timeline
  14. Remove the global filter
  15. Find the BiteBanana type in the "Resulting Types" section of the modal and click on the filter+ icon
  16. A new filter row should appear below the type
  17. Add the filter "parameter biteSize equals 1" and verify that the BiteBanana activity appears on the timeline
  18. Change the filter to "parameter biteSize equals 2" and ensure that the BiteBanana activity does not show up on the timeline
  19. Test the other fields (Name, subsystem, etc) and operators (does not equal, includes, etc) within the dynamic/global/resulting type filters and ensure that they function as you would expect
  20. Make sure you have some filters applied and then save your UI view and reload the page
  21. Re-open the activity layer filter modal and ensure that all filters appear as they did before reload and that the correct activities appear on the timeline

Resources

  1. Create a plan and open it
  2. Simulate
  3. Create a new timeline row and edit the row
  4. Click on the + button to the right of the "Resource Layers" section in the editor
  5. A new resource layer should appear in the editor
  6. Click on the "select resource" dropdown
  7. Click on "/fruit"
  8. /fruit resource should appear on the timeline in the new row
  9. Select a different resource and verify that only one resource can be selected at a time
  10. Click on the x-range icon (next to the line layer icon)
  11. The resource should display as an x-range plot (note that most resources can only be plotted as true line OR true x-range so this switching is a little odd in some ways since it won't just magically render in both modes without some tweaking)

External Events

  1. Add external events to your local aerie instance if not already applied (can upload an example set from the UI repo e2e-test folder)
  2. Create a plan and open it
  3. Associate the external events with the plan
  4. Create a new timeline row and edit the row
  5. Click on the + button to the right of the "Event Layers" section in the editor
  6. A new event layer should appear in the editor
  7. Click on the "Select Event Types" dropdown
  8. Verify that multiple event types can be selected and that they appear on the timeline (if you made a plan that spans the event file time range, if not, skip this verification)

Activity Types Browser

  1. Create a plan
  2. Open the "Activity, Resource, Event Types" pane
  3. Test the addition of activity types to the timeline via menu, single drag, bulk drag and ensure that when possible, the filter you have entered (and or the subsystem you use if you have created any manually via hasura) are used as dynamic activity filters. For example, type in "banana" into the "filter activity types" input box and then add the filter to a row via menu or dragging. Then examine the activity layer filter and ensure that there is the dynamic filter "where type includes banana".

Migration

  1. Create a plan
  2. Click the view menu in the top right of the plan header UI
  3. Click on the "Upload View" menu option
  4. Upload various views made prior to this PR and verify that the views appear the same when compared in the old UI and the new UI

TODO:

  • Blocked: Awaiting design – Implement basic resource and event filtering
  • Test scheduling goal ID
  • Performance testing
  • Provide a mechanism for a user to easily specify that they want all activity types All Activities timeline #973
  • UI View migration
  • Write tests
  • Code review
  • Documentation
  • Make issue for using the new default activity args cache instead of calling getEffectiveArguments every time a user clicks on an activity

@AaronPlave AaronPlave changed the title Feature/advanced timeline item filtering Advanced Timeline Filtering Dec 2, 2024
@AaronPlave AaronPlave self-assigned this Dec 2, 2024
@AaronPlave AaronPlave force-pushed the feature/advanced-timeline-item-filtering branch from 6d887d8 to 7b25a40 Compare December 2, 2024 17:24
@dandelany
Copy link
Collaborator

Met with @AaronPlave and @lklyne about remaining tasks this morning. All of the new functionality for advanced filtering of activities is done on this branch and available for testing on our (JPL internal) aerie-test instance. However there is still a bit of design + UI work to be done on integrating this work with the row editor in the Timeline Editor pane. Specifically:

  • When the user is editing a row in the Timeline Editor, the layers on the row are now split into three categories: Activity Layers, Resource Layers and Event Layers
  • The UI for these sections needs to be refined - currently some inconsistencies in design & behavior.
  • The 3 types of layers have some common characteristics but some intentional differences, we should have a UI that keeps them as consistent-looking as possible while accounting for their differences
  • Things in common between Activity, Resource and Event layers:
    • All have buttons to add layer, delete layer, and delete all layers
    • All have the ability to set a user-defined layer name, plus should have some space to optionally show a subtitle, such as the name of the underlying resource or a summary of activity filters.
    • All have a color swatch for setting the layer color
  • Differences between layer types:
    • Activity layers now select a set of filters via the new modal window, instead of just a list of activity types
      • Per original design, they should also show the number of matching activity instances in the plan, if not too difficult to implement
    • Resource layers now only allow users to select a single resource per layer. Formerly the UI "allowed" selecting multiple but it didn't really work correctly.
      • they also have a settings icon + popover, and a selector for chart type (line vs. x-range)
    • Event layers allow the user to select multiple [external] event types to show on a single layer, unlike resources.

@lklyne will work on a unified UI design for handling these different types in a consistent way + then handoff to @AaronPlave to implement on this branch.

@AaronPlave AaronPlave force-pushed the feature/advanced-timeline-item-filtering branch from f544330 to b7fbd44 Compare January 6, 2025 18:52
on:hide={() => (manualInputOpen = false)}
>
<div class="manual-types-menu">
{#if filteredActivityTypes.length > 0}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just something to think about: this could potentially use a component for each item that is shared with the SearchableDropdown to reuse some of the dropdown logic for selecting individual and selecting all items that match what is searched.

@@ -53,7 +54,7 @@
}
});

function padLeftSlot() {
async function padLeftSlot() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need to become an async function?

@@ -65,19 +66,17 @@
}
// because the content of the slot might not have been fully rendered by the time this function is called
// we must kick it out to a timeout to wait for it to be rendered
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this timeout no longer needed? If so, we can probably take out the comment.

await activityLayerEditor
.locator('.timeline-layer-editor')
.first()
.getByLabel('activity-filter-builder-trigger')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little hesitant about using the label as a way to select the button in the test. Anyway this could be getting the button by text or role?

bind:this={filterMenu}
>
<button
aria-label="activity-filter-builder-trigger"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't aria-labels be more human readable? If this is for a test, can we use the data-testId attribute instead?

uniqueDirectives.push(directive);

// Gather spans for directive since we always show all spans for a directive
// TODO aplave does not know if this is a good move, needs investigation and potentially options
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

return included;
}

export function getMatchingTypesForActivityLayerFilter(filter: ActivityLayerFilter | undefined, types: ActivityType[]) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this function is being used outside, but not matchesDynamicFilter. Would it be possible to write tests for this instead of matchesDynamicFilter?

});
}

export function directiveOrSpanMatchesDynamicFilters(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also potentially doesn't need to be exported

}

// TODO try consolidating with the function above
export function typeMatchesDynamicFilters(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially doesn't need export

}, true);
}

export function matchesDynamicFilter(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might not need an export, although there is a test for it.

});

test('Deleting an simulation template should remove it from the list of templates', async () => {
await plan.selectSimulationTemplateByName('Template 1');

await page.getByRole('button', { name: 'Set Template' }).click();
await plan.panelSimulation.locator('div[name="Set Template"]').click();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's no longer possible to get this by role?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants