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

Feature/#204 consolidate search and multiselect #301

Merged
merged 68 commits into from
Jun 23, 2021

Conversation

iguannalin
Copy link
Contributor

See issue: #204

clemiller and others added 30 commits April 22, 2021 15:46
…reated object in which to store stix bundle changes
…ded autosize and mode="side" setting to mat-drawer to push matrix to the side when sidebar is open
…nd updated function of search icon in toolbar
…le component search button, and in the layer upgrade component sidebar done button
…arch button" -- search icon in toolbar should function as before sidebar updates (this is to keep the changes separate, as they are different issues)

This reverts commit 222d9dc.
…he sidebar, depending on where it was clicked from
…function, as it was throwing an error trying to convert undefined to lower case
…pdating with my changes to set the sidebar content
…ebar; Updated search feature to query StixObjects also--the search queries the same decreasing number of StixObjects until either: 1) there's no results, or 2) the query clears out (is an empty string ""); it was too slow to repeat the search with a new set of StixObjects each time, so there's getStixData() and getStixResults()
…f query is completely different from the last query
@iguannalin
Copy link
Contributor Author

iguannalin commented Jun 16, 2021

We may want to look into possible performance enhancements to avoid visual latency in that part of the application.

I should note that performance is always slightly better in the actual live deployment (presumably because of optimizations in the production build), so we may want to do more testing of the performance just in case I'm wrong in my method of measurement. Probably the best way to test performance is to check out the previous version (4.3) and run it in parallel on a different port and take the average of 10 observances of mousenter or something to that effect. This will produce closer conditions than in my testing.

@isaisabel, I looked into this and tested the performance of the mouseenter event in 4 environments:

  1. locally, develop branch (65.78ms)
  2. locally, master branch (78.78ms)
  3. locally, feature/# 204 branch (76.59ms)
  4. deployed v4.3 (20.08ms)

I tested this by running the performance profiling, reloading the page, and hovering over the 4 techniques in the top left corner after the matrix loads. It seems to take consistently ~60-80ms for the mouseenter event when running locally (I ran these about 2-3 times), and significantly less at 20ms in the deployed version on the mitre-attack site.

If I'm doing this right and understanding you correctly, I'm not sure if there's a significant lag in the mouseenter/hover because of the changes in this feature branch. One thing I do think concerns me about performance when working on this feature though, was the loading of all the techniques in the search component sidebar (since it now loads & searches techniques across domains and all techniques).

Screenshots:

  1. Screen Shot 2021-06-16 at 9 32 16 AM
  2. Screen Shot 2021-06-16 at 9 32 33 AM
  3. Screen Shot 2021-06-16 at 9 39 24 AM
  4. Screen Shot 2021-06-16 at 9 31 10 AM

Copy link
Contributor

@isaisabel isaisabel left a comment

Choose a reason for hiding this comment

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

I actually have found one major bug in my second review. When switching between layers of different domains, the contents of the sidebar does not update properly until a query is added.

Steps to reproduce:

  1. Open two layers, one Enterprise and one Mobile
  2. Open the sidebar on both tabs
  3. Switch back and forth between the two tabs. The counts and listed objects should change when switching tabs since the data under each domain is different, but this does not happen. Only when a query is inputted do the objects listed refresh.

Aside from that everything looks great, and the added denounce definitely makes the search input more performant.

Copy link
Contributor

@isaisabel isaisabel left a comment

Choose a reason for hiding this comment

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

Looks great! I pushed a commit which updates the help page documentation which was pretty old for this feature (it didn't mention mitigations which were not originally part of the interface and generally needed a touch-up).

@isaisabel isaisabel merged commit 8819266 into develop Jun 23, 2021
@clemiller clemiller deleted the feature/#204-consolidate-search-and-multiselect branch February 17, 2022 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consolidate Search and Multiselect Interfaces
3 participants