Skip to content

Conversation

@keithchong
Copy link
Contributor

@keithchong keithchong commented Jan 27, 2022

Signed-off-by: Keith Chong [email protected]

About this PR:

Addresses #7183

  • This zoom implementation is strictly transformation, which is option/solution A. Panning is limited to scrolling of the enlarged rectangular area, and isn't truly panning of a view-box typical of SVG graphics. For option B, we could convert to all SVG but its performance degrades for many rendered objects. I've investigated this more and there is a bit of work involved.

  • Zoom level is not made a view preference on the details page so that the UI in multiple browser tabs can have different zoom values. Drawback is that the zoom level will reset each time you exit and enter the details page, and it's different behavior than the grouped-node button. However, zoom level is maintained when switching from tree view to network view and vice versa.

  • Grouped-node button moved to floating graph toolbar. Zoom buttons similar to the ones seen in the Argo Workflows UI.

  • CSS and tool bar can be moved to argo-ui, but is not part of this PR

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • Optional. My organization is added to USERS.md.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).

@keithchong
Copy link
Contributor Author

keithchong commented Jan 27, 2022

Zoom out 40%:
zoom01

Zoom in 150%:
zoom02

Group nodes enabled:
zoom03

@codecov
Copy link

codecov bot commented Jan 27, 2022

Codecov Report

Merging #8290 (e35bfdf) into master (0b2e585) will increase coverage by 0.29%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8290      +/-   ##
==========================================
+ Coverage   41.63%   41.93%   +0.29%     
==========================================
  Files         174      176       +2     
  Lines       22881    22992     +111     
==========================================
+ Hits         9527     9641     +114     
+ Misses      11990    11966      -24     
- Partials     1364     1385      +21     
Impacted Files Coverage Δ
server/cache/cache.go 68.00% <0.00%> (-5.34%) ⬇️
util/settings/settings.go 47.08% <0.00%> (-0.06%) ⬇️
util/buffered_context/buffered_context.go 100.00% <0.00%> (ø)
util/crypto/crypto.go 44.82% <0.00%> (ø)
reposerver/repository/repository.go 58.08% <0.00%> (+0.23%) ⬆️
util/kustomize/kustomize.go 69.79% <0.00%> (+1.99%) ⬆️
util/oidc/provider.go 5.88% <0.00%> (+5.88%) ⬆️
cmpserver/plugin/plugin.go 51.92% <0.00%> (+15.98%) ⬆️
util/oidc/oidc.go 40.67% <0.00%> (+21.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b2e585...e35bfdf. Read the comment docs.

@keithchong
Copy link
Contributor Author

Hi @alexmt, @rbreeze, @ciiay, please have a look. FYI, @ciiay, I changed the button that you contributed.

Copy link
Member

@rbreeze rbreeze left a comment

Choose a reason for hiding this comment

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

LGTM! I'd like @alexmt to confirm before merging if he has time

Copy link
Collaborator

@alexmt alexmt left a comment

Choose a reason for hiding this comment

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

That is just awesome!

Can we persist zoom settings in user preferences for consistency? This definitely can be done later.

LGTM

@alexmt alexmt merged commit f387ab8 into argoproj:master Jan 27, 2022
@saumeya
Copy link
Contributor

saumeya commented Jan 28, 2022

This is such a cool feature!

pasha-codefresh pushed a commit to pasha-codefresh/argo-cd that referenced this pull request Feb 2, 2022
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.

4 participants