Skip to content

Conversation

@oatkiller
Copy link
Contributor

@oatkiller oatkiller commented Jul 19, 2020

Co-authored-by: Brent Kimmel brent.kimmel@elastic.co
closes https://github.com/elastic/endpoint-app-team/issues/579

Summary

  • Center the origin node
  • Nodes appear selected when they are selected. also the aria attributes are working.
  • Reposition the submenu when the user pans.

Note: this doesn't include the following. We need a follow up PR

  • origin node starts off selected

Generally works:
image

Origin is in the center, selecting nodes works, and the submenu is correctly repositioned when the graph moves.

centering_and_submenu_position_and_selecting_nodes_and_aria_works mov

panels still work
panels_work mov

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@oatkiller oatkiller requested review from a team as code owners July 19, 2020 05:40
@oatkiller oatkiller added release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels Jul 19, 2020
Copy link
Contributor

@kqualters-elastic kqualters-elastic left a comment

Choose a reason for hiding this comment

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

👍 code looks fine, the way the process looks not so sure

@oatkiller oatkiller force-pushed the trigger-proces branch 2 times, most recently from 12bceab to b0d689b Compare July 20, 2020 19:25
Copy link
Contributor

Choose a reason for hiding this comment

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

❔ Assume by position in this comment you mean translation: Vector2 in the args as below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

typescript error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
* Return a clone of `model` with all positions incremented by `position`.
* Return a clone of `model` with all positions incremented by `translation`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

remove TODO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO elaborate on how this works in comments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this isn't defined yet. we dont have a way to know if its a trigger? TODO add comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

by using const next: ResolverUIState, typescript will complain about excess properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

show the special terminated trigger cube.

Copy link
Contributor

Choose a reason for hiding this comment

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

🔴 still see our little experiment here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
useLayoutEffect,

CI freaked out on this

@ghost
Copy link

ghost commented Jul 21, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #72382 updated]

  • Start Time: 2020-07-21T14:29:04.543+0000

  • Duration: 4 min 56 sec

oatkiller added 3 commits July 21, 2020 13:23
also renames 'isProcessOrigin' params in resolver theme to be
'isProcessTrigger'.

NB: this does not use the trigger node styling
UI state changes:
* rename `activeDescendantId` to `ariaActiveDescendant`. it has the
nodeID (aka entity_id) of the aria active descendant
* rename `processEntityIdOfSelectedDescendant` to `selectedNode`. it has
the nodeID (aka entity_id) of the selected node
* aria html attributes were wrong (this also effected styling.)
@oatkiller oatkiller mentioned this pull request Jul 21, 2020
7 tasks
@oatkiller
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

async chunks size

id value diff baseline
securitySolution 7.3MB +1.1KB 7.3MB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@oatkiller oatkiller merged commit b930cef into elastic:master Jul 21, 2020
@oatkiller oatkiller deleted the trigger-proces branch July 21, 2020 21:47
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 22, 2020
* master: (23 commits)
  Stabilize closing toast (elastic#72097)
  stabilize failing test (elastic#72086)
  Stabilize filter bar test (elastic#72032)
  Unskip vislib tests (elastic#71452)
  [ML] Fix layout of anomaly chart tooltip for long field values (elastic#72689)
  fix preAuth/preRouting mocks (elastic#72663)
  [Security Solution] Hide KQL bar (all pages) and alerts filters (Detections) when Resolver is full screen (elastic#72788)
  [Uptime] Rename Whitelist to Allowlist in parse_filter_map (elastic#71584)
  [Security Solution] Fixes exception modal not loading content (elastic#72770)
  [Security Solution][Exceptions] - Require non empty entries and non empty string values in exception list items (elastic#72748)
  [Detections] Add validation for Threshold value field (elastic#72611)
  [SIEM][Detection Engine][Lists] Adds version and immutability data structures (elastic#72730)
  [Security Solution][Detections] Validate file type of value lists (elastic#72746)
  [pre-req] New Component Layout proposal (elastic#72385)
  [ML] do not throw an error when agg is not supported by UI (elastic#72685)
  [Resolver] Origin process (elastic#72382)
  [Ingest Manager] Allow to force unenroll from the UI (elastic#72386)
  skip 6.8 branch when triggering baseline-capture builds (elastic#72706)
  [CI] In-progress PR comments (elastic#72211)
  Fix sorting of scripted string fields (elastic#72681)
  ...
oatkiller pushed a commit to oatkiller/kibana that referenced this pull request Jul 23, 2020
Co-authored-by: Brent Kimmel <brent.kimmel@elastic.co>

* Center the origin node
* Nodes appear selected when they are selected. also the aria attributes are working.
* Reposition the submenu when the user pans.
oatkiller pushed a commit to oatkiller/kibana that referenced this pull request Jul 23, 2020
Co-authored-by: Brent Kimmel <brent.kimmel@elastic.co>

* Center the origin node
* Nodes appear selected when they are selected. also the aria attributes are working.
* Reposition the submenu when the user pans.
oatkiller pushed a commit that referenced this pull request Jul 23, 2020
Co-authored-by: Brent Kimmel <brent.kimmel@elastic.co>

* Center the origin node
* Nodes appear selected when they are selected. also the aria attributes are working.
* Reposition the submenu when the user pans.
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 23, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

oatkiller pushed a commit that referenced this pull request Jul 23, 2020
Co-authored-by: Brent Kimmel <brent.kimmel@elastic.co>

* Center the origin node
* Nodes appear selected when they are selected. also the aria attributes are working.
* Reposition the submenu when the user pans.

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@kibanamachine kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jul 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes v7.9.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants