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

Action menu: sort items, show in resource details page #11008

Merged
merged 15 commits into from Nov 8, 2022

Conversation

alexef
Copy link
Member

@alexef alexef commented Oct 20, 2022

I notice that every time I open the burger menu, the resource customization actions show up in random order. This sorts them alphabetically.

Additionally, to avoid an extra click, I'm adding the menu to the resource details page (next to SYNC and DELETE buttons).

Before

Screenshot 2022-10-20 at 10 15 13

After

Screenshot 2022-10-20 at 10 35 54

Screenshot 2022-10-20 at 11 37 26

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).

    So that the same action shows up in the same position every time the drop down is opened

Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
@codecov
Copy link

codecov bot commented Oct 20, 2022

Codecov Report

Base: 45.60% // Head: 45.60% // No change to project coverage 👍

Coverage data is based on head (2faf05c) compared to base (4d6d204).
Patch has no changes to coverable lines.

❗ Current head 2faf05c differs from pull request most recent head 9a9df1c. Consider uploading reports for the commit 9a9df1c to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #11008   +/-   ##
=======================================
  Coverage   45.60%   45.60%           
=======================================
  Files         239      239           
  Lines       28973    28973           
=======================================
  Hits        13214    13214           
  Misses      13940    13940           
  Partials     1819     1819           
Impacted Files Coverage Δ
util/settings/settings.go 51.25% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rbreeze
Copy link
Member

rbreeze commented Oct 28, 2022

One small suggestion at first glance:
It appears there is no margin between the existing button and the new three dots button to show the menu. Can you add a few pixels of margin?

@alexef
Copy link
Member Author

alexef commented Oct 28, 2022

@rbreeze I can do that, but I have a different idea. What if (on the details page) instead of a drop down, I generate gray rounded buttons (looking the same as sync and diff). Then we avoid one extra click to expand the drop down. the disabled ones can be rendered as disabled or not rendered at all.

What I'm afraid is only if the list becomes too large, on smaller screens it may look weird, overlap with the Resource name on the left.

Should I give it a try?

@alexef
Copy link
Member Author

alexef commented Nov 3, 2022

@rbreeze I've added the margin.

Now it looks like this:
Screenshot 2022-11-04 at 00 19 31
Screenshot 2022-11-04 at 00 17 41

Something odd sometimes it loads to the left, other times to the righ. I've checked argo-ui and this doesn't seem to be configurable.

rbreeze and others added 2 commits November 4, 2022 11:18
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
Copy link
Member

@blakepettersson blakepettersson left a comment

Choose a reason for hiding this comment

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

LGTM! 🙌

@alexef
Copy link
Member Author

alexef commented Nov 7, 2022

E2E failures can't be related to changes in PR (as they passed before merge with master).

@rbreeze can you have another look? 🙏

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!

@rbreeze rbreeze enabled auto-merge (squash) November 8, 2022 17:41
@rbreeze rbreeze merged commit 149926d into argoproj:master Nov 8, 2022
@alexef alexef deleted the action-menu branch November 21, 2022 08:18
ashutosh16 pushed a commit to ashutosh16/argo-cd that referenced this pull request Nov 23, 2022
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
Co-authored-by: Remington Breeze <remington@breeze.software>
emirot pushed a commit to emirot/argo-cd that referenced this pull request Jan 27, 2023
Signed-off-by: Alex Eftimie <alex.eftimie@getyourguide.com>
Co-authored-by: Remington Breeze <remington@breeze.software>
Signed-off-by: emirot <emirot.nolan@gmail.com>
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.

None yet

3 participants