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

Close Actions after Action was clicked #615

Closed
raimund-schluessler opened this issue Oct 1, 2019 · 19 comments
Closed

Close Actions after Action was clicked #615

raimund-schluessler opened this issue Oct 1, 2019 · 19 comments
Labels
discussion Need advices, opinions or ideas on this topic enhancement New feature or request feature: actions Related to the actions components

Comments

@raimund-schluessler
Copy link
Contributor

raimund-schluessler commented Oct 1, 2019

I have the following component creating a dropdown with popovermenu items:

<template>
	<div class="menu-icon">
		<div v-click-outside="close" class="header-icon icon-more" @click="toggle" />
		<div :class="{ 'open': open }" class="popovermenu">
			<popover-menu :menu="menu" />
		</div>
	</div>
</template>

<script>
import PopoverMenu from 'nextcloud-vue/dist/Components/PopoverMenu'
import ClickOutside from 'vue-click-outside'

export default {
	components: {
		PopoverMenu,
	},
	directives: {
		ClickOutside,
	},
	props: {
		menu: {
			type: Array,
			required: true,
		}
	},
	data: function() {
		return {
			open: false,
		}
	},
	methods: {
		toggle: function() {
			this.open = !this.open
		},
		close() {
			this.open = false
		},
	}
}
</script>

I want to close the dropdown when an item is clicked. A simple click handler on the popover-menu doesn't work, because the click is prevented in popover-menu. The alternative could be to implement a two-way sync for this.open similar to #278, and closing the menu from the parent component after action is handled, but this gets messy quickly when you have multiple dropdowns in the parent component (you need to track which dropdown is opened).

I propose to emit a click event like this.$emit('clickedAction) in the popover-menu item which the Dropdown component can process if necessary using `@clickedAction="close".

@raimund-schluessler raimund-schluessler added feature: popover Related to the popovermenu component enhancement New feature or request labels Oct 1, 2019
@skjnldsv
Copy link
Contributor

skjnldsv commented Oct 1, 2019

Argh, we deprecated the popover menu. In favour of the actions component. Could you migrate to it?

@raimund-schluessler
Copy link
Contributor Author

Argh, we deprecated the popover menu. In favour of the actions component. Could you migrate to it?

Oh, I wasn't aware of this. I will have a look.

@skjnldsv
Copy link
Contributor

skjnldsv commented Oct 1, 2019

@raimund-schluessler ping me if you need help! But our doc is really awesome now :)
https://nextcloud-vue-components.netlify.com/

@raimund-schluessler
Copy link
Contributor Author

@skjnldsv Well, the Actions component has the same problem. When I want to close the Actions menu after an Action was clicked, I need to do this from the parent component via the open property. As said above, this is cumbersome when you have multiple Actions in one component.

Having it self-contained inside the Actions component (at least optional) would be a huge improvement in my opinion.

@raimund-schluessler raimund-schluessler changed the title Close dropdown with popover-menu items on click Close Actions after Action was clicked Oct 1, 2019
@raimund-schluessler raimund-schluessler added feature: actions Related to the actions components and removed feature: popover Related to the popovermenu component labels Oct 2, 2019
@skjnldsv
Copy link
Contributor

skjnldsv commented Oct 2, 2019

When I want to close the Actions menu after an Action was clicked

Could you give me a use case on why this is required?
I don't see how this is really an advantage. :)

@raimund-schluessler
Copy link
Contributor Author

raimund-schluessler commented Oct 2, 2019

Could you give me a use case on why this is required?

I have a table and every row has an Actions menu with actions to delete or edit the row. When I click on edit, the table cells show input elements and the input in the first cell is focused automatically. So I can start typing immediately, but the Actions menu is still opened, overlapping the last inputs. Hence, I need to click somewhere to close the menu.

Edit: Just as in the files app, clicking rename on a file opens an input and closes the menu.

Using :open="..." on the Actions menu would be a way to close it after clicking edit, but the problem here is, that I have multiple Actions in the component (one in every table row), so I would need to track which Actions menu is currently opened and handle the @open and @close events of the Actions component to sync the state.

@raimund-schluessler
Copy link
Contributor Author

In case you have a better idea how to do this, let me know :)

@skjnldsv
Copy link
Contributor

skjnldsv commented Oct 2, 2019

Ah yes, you're right! It totally make sense for those kind of actions! :)

so I would need to track which Actions menu is currently opened and handle the @open and @close events of the Actions component to sync the state.

nly the one the user clicked on.
All the other will be caught by the click outside :)

So if you only take care of a local open variable and use :open.sync="open" and add this.open = false in the actions you want it to be closed, it should be fine.

I actually prefer something like this because it gives the dev more control. If we have this as default in the actions component, it will only apply to either all or none. So if you have actions you want it closed after click, but others you want it to stay opened, you will need to have it per actions anyway, which goes to the same point of implementing a this.open = false on each action handler :)

@raimund-schluessler
Copy link
Contributor Author

So if you only take care of a local open variable and use :open.sync="open" and add this.open = false in the actions you want it to be closed, it should be fine.

I actually prefer something like this because it gives the dev more control. If we have this as default in the actions component, it will only apply to either all or none. So if you have actions you want it closed after click, but others you want it to stay opened, you will need to have it per actions anyway, which goes to the same point of implementing a this.open = false on each action handler :)

I tried exactly this, but none of the popovers will open anymore. This is because the click outside of the other Actions sets the open state to false, it is synced to the parent component and prevents opening every Actions menu.

@skjnldsv
Copy link
Contributor

skjnldsv commented Oct 2, 2019

@raimund-schluessler Well I'm suggesting one open state per Actions component, so it should not interfer :)

@raimund-schluessler
Copy link
Contributor Author

This becomes really cumbersome and basically impossible when you use the actions in a v-for loop.

@skjnldsv
Copy link
Contributor

skjnldsv commented Oct 2, 2019

AH yes! Now I get it!
Hum, that's a tricky one :)

How do you suggest we implement this into the components?

@korelstar
Copy link
Contributor

I had the same problem when using Actions in AppNavigationItem and solved this by creating a separate component for the part that uses Actions. Then, there is only one open to be synced in each component.

@raimund-schluessler
Copy link
Contributor Author

I had the same problem when using Actions in AppNavigationItem and solved this by creating a separate component for the part that uses Actions. Then, there is only one open to be synced in each component.

That would be one option, but creating a wrapper component for the Actions just to scope the open variable seems to be a bit over the top.

How about we this.$emit('closeActions') in the ActionItems, listen to it in the Actions component and close the menu when this event is received. A :closeActions="true" on the ActionItems could toggle whether we emit the event. I can send a PR if you agree on this approach :)

@skjnldsv
Copy link
Contributor

skjnldsv commented Oct 2, 2019

I would go for something more specific like <ActionButton close-after-click> or something like that?
I'm not really fond of this solution though, it feels a bit hackish, I really wonder if we cannot do otherwise. I feel like we're implementing a feature for a very specific use case: action within a v-loop.

But in the other hand, other could benefit from it too even if not in a loop as they won't have to implement their own this.open = false... 🤔

What do you think ? No other ideas? If not, let's do this! 💪 :)

@skjnldsv skjnldsv added the discussion Need advices, opinions or ideas on this topic label Oct 2, 2019
@raimund-schluessler
Copy link
Contributor Author

I would go for something more specific like <ActionButton close-after-click> or something like that?

Fine with me.

I'm not really fond of this solution though, it feels a bit hackish, I really wonder if we cannot do otherwise. I feel like we're implementing a feature for a very specific use case: action within a v-loop.

But in the other hand, other could benefit from it too even if not in a loop as they won't have to implement their own this.open = false... 🤔

To me, the this.open = false felt hackish, since you kind of do a round-trip from the ActionButton (child) to the outer component (parent) to alter the state in the ActionsMenu. But I guess this is a matter of taste 😉

I will create a PR, so we can discuss on the code if we like it.

@skjnldsv
Copy link
Contributor

skjnldsv commented Oct 2, 2019

I will create a PR, so we can discuss on the code if we like it.

You rock! Thanks for taking over! :)
Well, to be fair we also need to move on, since there is no real solution to this for now. Let's go for this, should be fine ;)

@raimund-schluessler
Copy link
Contributor Author

PR in #622.

@raimund-schluessler
Copy link
Contributor Author

Was closed with #622.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Need advices, opinions or ideas on this topic enhancement New feature or request feature: actions Related to the actions components
Projects
None yet
Development

No branches or pull requests

3 participants