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

Bug or feature? Toolbar's data-toolbar-reorder deletes callbacks #2200

Closed
RunDevelopment opened this issue Feb 5, 2020 · 6 comments
Closed
Labels

Comments

@RunDevelopment
Copy link
Member

While writing #2199, I noticed that this line reorders the buttons and can also be used to disable buttons since all buttons which aren't in the list won't show up.

This has been in there since #891, so @mAAdhaTTah: Is it a bug or a feature?

@mAAdhaTTah
Copy link
Member

I think Imma go with feature. I didn't think about that use case when I did it in the first place, but I think if the user expresses their intent as [a, b], then it should render a, then b. Adding additional automatically seems to violate their intent. Which kinda suggests we don't need #2199 as the user can use order and keep the one(s) they want.

@RunDevelopment
Copy link
Member Author

Feature it is then. But we still need #2199 because data-toolbar-reorder is actually a global reordering/selection of buttons and will be applied to all elements because it is currently implemented as a body attribute (not inherited). So it wouldn't be possible to disable a single button which is the issue #2199 is trying to solve albeit in a more general manner.

That being said, if data-toolbar-reorder were to be inherited, it would also solve the issue motivating #2199.

@mAAdhaTTah
Copy link
Member

I was actually going to suggest that. We do that sort of "inheriting" thing elsewhere, so that seems more in line with the pattern elsewhere.

I am also wondering if it makes sense to not have the toolbar appear on the inline code blocks (e.g. blocks without a parent pre element) at all, cuz that's usually used for small words or snippets that don't need the toolbar.

@RunDevelopment
Copy link
Member Author

Alright, then I'll do that.

I am also wondering if it makes sense to not have the toolbar appear on the inline code blocks

Uhm. It doesn't.

@mAAdhaTTah
Copy link
Member

Ahh, I misinterpreted this comment from the original request:

Copy to Clipboard - Would really prefer to disable it on single line code blocks.

But I get it now. So yeah, updating to inherit would be good.

@RunDevelopment
Copy link
Member Author

No problem. I'll close the issue as the initial question has been answered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants