feat: adds menu for actions on containers #804

cdrage · 2022-11-09T20:43:42Z

feat: adds menu for actions on containers

What does this PR do?

This PR adds a "kebab" menu for the actions on containers

Screenshot/screencast of this PR

What issues does this PR fix or reference?

This menu was implemented while testing out #506

How to test this PR?

yarn watch and press the action button

Signed-off-by: Charlie Drage charlie@charliedrage.com

cdrage · 2022-11-09T20:47:57Z

Pushed this functionality in a new PR!

@benoitf

I've updated the code from your advice and implemented:

Updated UI for the menu
ESC now closes the menu
Clicking outside closes the menu

@vzhukovs

I tried implementing #800 but decided to use the tutorial from svelte here instead: https://svelte.dev/tutorial/actions

packages/renderer/src/lib/container/ContainerActions.svelte

cdrage · 2022-11-09T21:14:15Z

TODO:

Fix issue with kebab menu showing on container details
Fix issue with "toggle" not working when pressing the button again

packages/renderer/src/lib/container/ContainerActions.svelte

packages/renderer/src/lib/ui/util.ts

vzhukovs

Looks good to me

packages/renderer/src/lib/container/ContainerActions.svelte

benoitf

it looks like the kebab menu will be specific to ContainerActions and can't be used elsewhere ?

Probably we need instead of

{#if showMenu}
    <div id="dropdown"

<KebabMenu>
   <KebabMenuItem title="" onClick="" hidden=>

https://svelte.dev/repl/8e68120858e5322272dc9136c4bb79cc?version=3.5.1

cdrage · 2022-11-15T15:26:16Z

it looks like the kebab menu will be specific to ContainerActions and can't be used elsewhere ?

Probably we need instead of
{#if showMenu}
    <div id="dropdown"
<KebabMenu>
   <KebabMenuItem title="" onClick="" hidden=>
https://svelte.dev/repl/8e68120858e5322272dc9136c4bb79cc?version=3.5.1

Yes you're right! I'll make it modular so we can use this somewhere else. Makes it easier to implement the kebab menu on pod list, future sections, etc.

cdrage · 2022-11-15T20:51:22Z

@benoitf @vzhukovs Updated the PR and it's ready for review!

I've completed:

Not showing the kebab when viewing details
Made it as a component so we can re use this with Pods list, etc.

Please note the dropdownMenu=true and menu=true portions of the code.

Hopefully this is good Svelte code, and if it's not I'm open ears to any form of criticism as I'm still learning 💯

benoitf

Hello,

I still do believe the trash icon should be visible as it's a primary action I'm using as often as start/stop

packages/renderer/src/lib/container/ContainerActions.svelte

packages/renderer/src/lib/ui/DropdownMenu.svelte

benoitf · 2022-11-16T07:55:25Z

packages/renderer/src/lib/ui/DropdownMenu.svelte

+    <!-- Dropdown menu for all other actions -->
+    {#if showMenu}
+      <div
+        id="dropdown"
+        class="origin-top-right absolute right-0 z-10 m-2 rounded-md shadow-lg bg-zinc-700 ring-1 ring-black ring-opacity-5 divide-y divide-gray-600 focus:outline-none">
+        <slot />
+      </div>
+    {/if}


really good improvement over the previous version
esc key works, display is not collapsed on the right

mVnFNJbq3K.mp4

but the drop down should be displayed before the pointer when we click on elements at the bottom because while it works fine for upper rows, clicking on the rows near the bottom, the menu is displayed at the bottom and then we need another scroll. It should be displayed in that case at the opposite side

benoitf · 2022-11-16T08:00:15Z

packages/renderer/src/lib/ui/DropdownMenu.svelte

+    <!-- Dropdown menu for all other actions -->
+    {#if showMenu}
+      <div
+        id="dropdown"


id should not be hardcoded as we've multiple dropdown on the page

benoitf · 2022-11-16T08:10:43Z

packages/renderer/src/lib/ui/DropdownMenu.svelte

+    {#if showMenu}
+      <div
+        id="dropdown"
+        class="origin-top-right absolute right-0 z-10 m-2 rounded-md shadow-lg bg-zinc-700 ring-1 ring-black ring-opacity-5 divide-y divide-gray-600 focus:outline-none">


divide-violet-600 ?

it looks like also that the dark theme is not following the same state than on the navbar or on the container list
display: dark item, hovering = lighter

here we have a light gray

I noticed a small issue: rounded corner seems to be off when hovering the element

F13LWtAAfy.1.mp4

benoitf

the menu doesn't disappear when I click on an item

so for example clicking again will do an action on the next row which is not an expected behaviour

A4Kwhg3WRz.mp4

cdrage · 2022-11-16T17:42:12Z

I've updated the PR with the following:

Fix round / radius on border on buttons
Made the entire div an "on click" for the menu buttons, as there were "blindspots" where you can click on blank space
Changed colour of the menu to black to coincide with the general UI
On hover, show purple colour / change text, similar to the current button functionality
Close menu after clicking a button

@benoitf I've been having troubles implementing the dropdown menu to go either UP or DOWN, any ideas?

I was thinking if cursor is on tophalf of the screen when clicking, drop down, if the cursor is bottom half, drop up? Would that be a good way to implement it?

benoitf · 2022-11-17T09:34:33Z

I created a PR against your branch:
cdrage#1

cdrage · 2022-11-17T13:31:24Z

@benoitf Merged. Thank you so much for the help / insight! 🎉

### What does this PR do? * This PR adds a "kebab" menu for the actions on containers ### Screenshot/screencast of this PR  ### What issues does this PR fix or reference?  This menu was implemented while testing out containers#506 ### How to test this PR? `yarn watch` and press the action button  Signed-off-by: Charlie Drage <charlie@charliedrage.com>

Signed-off-by: Charlie Drage <charlie@charliedrage.com>

…er click Change-Id: I36b990a88546119180b39464f038ee7fb8d97499 Signed-off-by: Florent Benoit <fbenoit@redhat.com>

Signed-off-by: Charlie Drage <charlie@charliedrage.com>

benoitf

tested again and works as expected. 🎉

maybe we need to use a 'purple' color divider but it's not a 'request for changes'

if possible, merge it on Monday, better than on Friday :-)

cdrage requested review from vzhukovs and benoitf as code owners November 9, 2022 20:43

cdrage force-pushed the kebab branch from 4fc3444 to 09165d7 Compare November 9, 2022 20:47

cdrage commented Nov 9, 2022

View reviewed changes

packages/renderer/src/lib/container/ContainerActions.svelte Outdated Show resolved Hide resolved

cdrage force-pushed the kebab branch 2 times, most recently from e8c8fa1 to 37425d9 Compare November 9, 2022 20:51

benoitf reviewed Nov 9, 2022

View reviewed changes

packages/renderer/src/lib/container/ContainerActions.svelte Outdated Show resolved Hide resolved

benoitf reviewed Nov 9, 2022

View reviewed changes

packages/renderer/src/lib/ui/util.ts Outdated Show resolved Hide resolved

cdrage force-pushed the kebab branch 3 times, most recently from 0140dca to 8a7be98 Compare November 14, 2022 20:18

vzhukovs approved these changes Nov 15, 2022

View reviewed changes

benoitf reviewed Nov 15, 2022

View reviewed changes

packages/renderer/src/lib/container/ContainerActions.svelte Outdated Show resolved Hide resolved

benoitf reviewed Nov 15, 2022

View reviewed changes

vzhukovs mentioned this pull request Nov 15, 2022

feat: update registries editing page #747

Merged

cdrage force-pushed the kebab branch 3 times, most recently from dc34693 to c5e260e Compare November 15, 2022 20:48

cdrage force-pushed the kebab branch from c5e260e to 974d907 Compare November 15, 2022 21:07

benoitf reviewed Nov 16, 2022

View reviewed changes

packages/renderer/src/lib/container/ContainerActions.svelte Outdated Show resolved Hide resolved

benoitf reviewed Nov 16, 2022

View reviewed changes

packages/renderer/src/lib/ui/DropdownMenu.svelte Outdated Show resolved Hide resolved

benoitf reviewed Nov 16, 2022

View reviewed changes

cdrage force-pushed the kebab branch from 974d907 to f619284 Compare November 16, 2022 17:39

cdrage force-pushed the kebab branch 4 times, most recently from bf67680 to 60b637b Compare November 18, 2022 14:10

cdrage and others added 3 commits November 18, 2022 09:10

remove background, change kebab menu to end button

9a72663

Signed-off-by: Charlie Drage <charlie@charliedrage.com>

chore: change placement on top or on bottom depending on where the us…

625eb30

…er click Change-Id: I36b990a88546119180b39464f038ee7fb8d97499 Signed-off-by: Florent Benoit <fbenoit@redhat.com>

cdrage force-pushed the kebab branch from 60b637b to 35ab5b4 Compare November 18, 2022 14:10

background, revert to other icons

de0b271

Signed-off-by: Charlie Drage <charlie@charliedrage.com>

cdrage force-pushed the kebab branch from 35ab5b4 to de0b271 Compare November 18, 2022 14:11

benoitf approved these changes Nov 18, 2022

View reviewed changes

benoitf merged commit 7a440df into containers:main Nov 21, 2022

podman-desktop-bot added this to the 0.10.0 milestone Nov 21, 2022

cdrage mentioned this pull request Nov 21, 2022

feat: menu implementation #800

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: adds menu for actions on containers #804

feat: adds menu for actions on containers #804

cdrage commented Nov 9, 2022 •

edited

cdrage commented Nov 9, 2022

cdrage commented Nov 9, 2022 •

edited

vzhukovs left a comment

benoitf left a comment •

edited

cdrage commented Nov 15, 2022

cdrage commented Nov 15, 2022

benoitf left a comment

benoitf Nov 16, 2022

benoitf Nov 16, 2022

benoitf Nov 16, 2022

benoitf left a comment

cdrage commented Nov 16, 2022 •

edited

benoitf commented Nov 17, 2022

cdrage commented Nov 17, 2022

benoitf left a comment

feat: adds menu for actions on containers #804

feat: adds menu for actions on containers #804

Conversation

cdrage commented Nov 9, 2022 • edited

What does this PR do?

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

cdrage commented Nov 9, 2022

cdrage commented Nov 9, 2022 • edited

vzhukovs left a comment

Choose a reason for hiding this comment

benoitf left a comment • edited

Choose a reason for hiding this comment

cdrage commented Nov 15, 2022

cdrage commented Nov 15, 2022

benoitf left a comment

Choose a reason for hiding this comment

benoitf Nov 16, 2022

Choose a reason for hiding this comment

benoitf Nov 16, 2022

Choose a reason for hiding this comment

benoitf Nov 16, 2022

Choose a reason for hiding this comment

benoitf left a comment

Choose a reason for hiding this comment

cdrage commented Nov 16, 2022 • edited

benoitf commented Nov 17, 2022

cdrage commented Nov 17, 2022

benoitf left a comment

Choose a reason for hiding this comment

cdrage commented Nov 9, 2022 •

edited

cdrage commented Nov 9, 2022 •

edited

benoitf left a comment •

edited

cdrage commented Nov 16, 2022 •

edited