-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add missing class names from sidebar panels #12082
Comments
This was asked in the past and the answer provided was that developers should not rely on CSS selectors to customize panels and that filters and slots should be used instead ( #6057 (comment) ). May I please ask that you try the solution noted there and if you have a very specific use case similar to the comments on that issue, note them here and we discuss the best route forward. |
@designsimply I'm not seeing any "solution" on that comment. Could you please provide some examples of how we can customize these sidebar panels using JS? For example:
|
@elliotcondon Have you looked at using |
@elliotcondon Further reading: #11802 |
@chrisvanpatten would love to see some code examples please 🙏. |
@elliotcondon Our notifications must have crossed paths but check #11802 :) |
@chrisvanpatten I commented too fast 🤣. Thanks for the link. I've had a look and can see there are a few functions available including My goal here is not to get bogged down in specific use cases. Instead, I am asking WordPress to retain the same thinking that went into the previous generation admin. The current "classic editor" is amazing to work with (as a web developer) because it follows simple and intuitive naming conventions / HTML markup. I don't believe we should ask web developers to refine their HTML/CSS/JS skills for front-end, but then tell them they are "doing it wrong" in the back-end. Web developers are a creative bunch. And even after 8 years working on ACF, I am still amazed at the customizations I see in the admin area/editor. I hope we can add in these classNames, it really will help a lot of people. |
@elliotcondon what would be the technical case for hiding a panel via CSS where the JS option wouldn’t be sufficient? I also think generally we want to discourage people from styling Gutenberg’s built in components, and certainly the editor panels, from a plugin or other integration. Generally the recommendation has been to filter any panels you want to style and provide your own markup/bindings to the API/styles — aka replacing the panels Gutenberg provides rather than modifying them. |
@chrisvanpatten It is very clear that you want to discourage people from styling Gutenberg’s built in components. Thanks for your time. |
@elliotcondon just to be clear I’m only a third party volunteer and my word should not be taken as gospel. I’m a part of the team but don’t represent the opinions of the release leads. My comments are just based on having followed/contributed to Gutenberg for some time and having seen similar questions and topics come up in past issues. I know it is a bit frustrating because the Gutenberg mentality is often different from previous norms — it is a big mental shift — but seriously if I can help, or help build a good technical case for a change, I am totally happy to do it. I am here to problem solve! |
@chrisvanpatten Can you please take a quick look at my screenshot above and let me know your thoughts? WP core add classes to the sidebar panels for CSS styling. Why not add it to all, so we can all style? |
The crazy thing here is that I personally don't need this feature. I've just worked in this space for so long, that I am good at predicting what "minor changes" will provide "maximum happiness" to developers. |
Ya this seems closed pretty prematurely 🤷♂️ |
Thank you for the suggestion, @elliotcondon! For hiding these panels, you should definitely use the JS API. Any sort of CSS changes won't be reflected in the mobile native block editor, but using the JS API will be. For other styling, I'm quite wary of encouraging custom styling of individual builtin components. Apart from having the same issue as above, where it would only apply to the web-based editor, there's been a significant amount of effort put into making Gutenberg the base for a WordPress design language. While there's still some ways to go, you can look at the block editor today, and see the consistency across the interface, something that WordPress has historically struggled to do. Design languages aren't new, plenty of other projects use them: Apple’s Human Interface Guidelines, or Google’s Material Design are great examples. For developers, these kinds of standard design documents allow you to build really nice interfaces without needing a designer to lay everything out for you. For designers, you get to focus on the bigger UX issues, rather than needing to style the same button 100 times. To bring it back to this particular issue, if there's something in the interface that needs customising, then it probably points towards an issue with the design language itself, which should be addressed. So, barring any specific use cases for adding these classes, I'm going to leave this issue closed. As the design language (and sidebar extensibility) evolves in subsequent WordPress releases, we should ideally be able to build something that doesn't require plugin developers to make custom tweaks. |
This need for “code hygiene” is absurd and totally contrary to the most common ways of using WordPress. It seems that there is a need to control at all costs what can or can not be done. Why?! This is not a project for designers and developers, it is a project for people, and people should be able to screw up if they want. It's not like we're breaking things on purpose. We are raising complexity in simple things, and this will limit, a lot, what the average user can do. I think it's contrary to the philosophy of the WordPress project. |
There are 2 parts I'd like to unpack a little from a pure React based point of view, the CSS side, and the JS side. CSSI don't think this is good practice, and in 99% of cases a developer thinks this is necessary, it isn't. But, there's probably some super obscure thing that requires it anyway that nobody has thought about, so it should be implemented for that JSHooking in via JS on the other hand is a huge no no. Lets work based on the assumption of this code: jQuery( '#tomshidepostrevisionsbutton').onClick( (evt ) => {
jQuery( '.edit-post-last-revision__panel' ).removeClass('is-opened');
} ); At first appearances this would remove the
So things will not work as expected, this is because there is a fundamental misunderstanding going on that assumes that latching on to DOM nodes generated by React is a good thing, that's not how things work. Not because somebody declared so, as a style or preference, but because structurally that's not how this works, in the same way that you can't redirect a river to flow up a mountain at a 45* angle by just moving dirt around. So How Does it Actually Work?Internally, the panel component has state. It's a JS array that looks similar to this: this.state = {
opened: props.initialOpen === undefined ? true : props.initialOpen,
}; If you click on the panel, a function called Why This is Suboptimal, and What Gutenberg Should Actually DoSo what really needs to happen is a way to changed So that's what needs requesting. That the UI's state be stored in a Doing it via jQuery or vanilla JS DOM manipulation will never work, or be conflict free, it never has been. For example in the classic editor you could target the internal Instead, TinyMCE provided APIs that you were meant to use, e.g.: tinymce.get(“content”).setContent(“my custom content”); In Gutenberg however, that data is structured and stored in the data store, you dispatch updates as I mentioned before. So how do we do that? Via // clear out all the content
wp.data.dispatch( 'core/editor' ).resetBlocks([]);
// Add a block
let block = wp.blocks.createBlock( 'core/paragraph', { content: 'Hello World' } );
wp.data.dispatch( 'core/editor' ).insertBlocks( block ); Running In an ideal world, we would do something similar for the panel. I strongly recommend that people follow this course by the author of redux, it walks from the very basics building layer on layer to explain how and why things work the way they do, and the problems solved, yet it's a super short course: https://egghead.io/courses/getting-started-with-redux edit: We totally do have this for the panel! See comments below |
Additionally: As the code demonstrates, toggling the HTML class has no bearing on whether the panel is shown or not. The most you can do is make it always hidden even when it's open |
Sorry @tomjn, the prior comment about |
@alvarogois no worries, I think there's a learning opportunity here for all, and I learnt some things myself. I'm sure I wrote that reply 3x over each time reading code and adjusting ( I didn't know the panels had internal state, and assumed they used a data store, had to rewrite half the thing! ) |
A good example of the kind of thing I suggested was actually done in #11802 as a means of removing panels: wp.data.dispatch( 'core/edit-post').removeEditorPanel( 'taxonomy-panel-category' ) ;
wp.data.dispatch( 'core/edit-post').removeEditorPanel( 'taxonomy-panel-post_tag' );
wp.data.dispatch( 'core/edit-post').removeEditorPanel( 'featured-image' );
wp.data.dispatch( 'core/edit-post').removeEditorPanel( 'post-excerpt' );
wp.data.dispatch( 'core/edit-post').removeEditorPanel( 'discussion-panel' ); There may even be |
And it is indeed possible: wp.data.dispatch( 'core/edit-post').toggleEditorPanelOpened('post-excerpt') Running that will open and close the excerpt panel |
To further @tomjn point, such an API would allow to separate the application state from the current DOM representation, which means we can orchestrate different plugins interacting with better means, as well as future compatibility with the native mobile apps implementation, where otherwise it would be extremely opaque to the environment. |
I may be missing something but part of the original ask wasn’t just about the class names that represent state (is-opened, etc) but also giving each panel its own named class, like how the first 2 in the screenshot have These aren’t stateful classes, unless I’m missing something? Why can’t each panel have consistent classes like this? I feel like the discussion sidetracked on the stateful stuff, but these have nothing to do with state. 🤔 |
Thanks @jasonbahl. Indeed, this discussion got very sidetracked. My original question was never to discuss the |
The solution I was referring to was to use filters and slots because that was my takeaway from the comment! But I am not a developer and so I thought the link to the handbook in that comment would suffice. Apologies for making that assumption. I will re-open this and tag it with Aside: (this probably isn't right) but I did look through the handbook a bit and I found a section for filters at https://wordpress.org/gutenberg/handbook/extensibility/extending-blocks/#filters and I also saw that |
Dangit. I had this open in an old tab and missed the entire conversation after Elliot's first reply to me. Sorry for the noise! /me backs away slowly … |
So.... Q2. How can we style a field (wrapping element with label / select) with CSS? |
Wait, so this is staying closed? I’m still confused why there can’t be consistent output of styles without the need for filters. Why are the panels treated inconsistently? Still seems like an oversight, not something to offload to site owners/plugin devs. |
+1 on unique classes or IDs. |
Adding unique classes/IDs seems like a relatively simple/small thing to do that could have a big impact for some projects. If WP is supposed to be this totally free and open platform why ever have a philosophy to restrict users in any cases? I get the material design thing but you can at least override some of that stuff if you're implementing it usually. We're all consenting adults here. I'm with @elliotcondon on this one 100%. |
Has nobody opened a pull request yet? It's all fun piling in in agreement
but condemnation and outrage are neither persuasive nor does it write code.
A pull request that actually implements this is the next step for those who
want it, until then it's purely academic so let's see some constructive
research into how it might be done? Or code!
…On Sat, 24 Nov 2018 at 02:18, zrubinrattet ***@***.***> wrote:
Adding unique classes/IDs seems like a relatively simple/small thing to do
that could have a big impact for some projects. If WP is supposed to be
this totally free and open platform why ever have a philosophy to restrict
users in any cases? I get the material design thing but you can at least
override some of that stuff if you're implementing it usually. We're all
consenting adults here. I'm with @elliotcondon
<https://github.com/elliotcondon> on this one 100%.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12082 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADl59QRglGxbh4kLqa0CA3RMxvLqu-qks5uyKxsgaJpZM4YqFc4>
.
|
With all due respect, we’re trying to first have the issue not be closed. If the gatekeepers won’t even acknowledge it is in fact an issue, why waste time on a PR? If we can agree that what @elliotcondon was asking originally, that panels should have consistent classes output I think there’s a chance one of us would be able to work on a PR. But if we’re shut down just by presenting the issue, why would we feel encouraged to open a PR? I’ll give it a shot if it’s an open issue (probably not super soon though because I’ve got family in town) |
My own personal take is that there are a lot of people in here asking for classes, but I don’t think we’ve seen a single concrete explanation of what the classes would actually be used for, other than “we might need them in the future.” I think what would help reopen the ticket are some concrete ideas of how the classes would be used, and why the current methods of customisation aren’t sufficient to accomplish those needs. The talk has admittedly gotten a little philosophical, and steering it back to real technical use-cases would be valuable. |
The only example I can think of at the moment is adding company branding to a panels header, which sounds like marketing UI clutter. Everything else can be done by either wrapping the panel contents in a div with a class, or adding per item level classes to the panels contents, which would be more optimal from a CSS standpoint. As for reopening it, simply stating some variation of "I disagree" is ineffectual, and polarizes things, so you are probably doing more harm for your cause than good. Examples, use cases, and code on the other hand work, e.g. implementing it and showing an example use case. As of yet, nobody has actually made an effective case for this outside of hooking in with JS, which has been shown that it won't work. |
Shouldn't be too difficult to think of various usecases if you have ever seen realworld WordPress used as CMS with sometimes highly customized backend to guide users with absolutely no technical background to publish posts - some would call this 'democratize publishing'. General usecase: Plugins which customize admin/edit area like Adminimize with 200.000+ active installs. Realworld usecase (simplified), client requirements:
Solution for Classic Editor:
Note: "+ Add New Category" has been clicked in both screenshots: Similar solution for custom taxonomies boxes, #submitdiv ... imagine that yourself. Overall development time for above requirements: 15 mins. Used toolchain: Browser inspector, text editor, Solution for Gutenberg? Maybe somebody can post a simple solution for category box in Gutenberg for above mentioned client requirements, would like to study and learn, thank you. Besides that: +1 on unique classes or IDs. |
I'd love to add unique classes or IDs. One reason is for consistency as @elliotcondon said. I noticed other issues as well: Take the Page Attributes meta box as an example. The CSS is following BEM style, but we can't find the class for the block, but inside elements. In details, we have only Another reason is it's nearly impossible to target a meta box. For example, there's no difference between taxonomies meta boxes. We can't differentiate built-in Category meta box vs. custom taxonomy meta box. The reason we need to target a meta box is: in our situation, we want to perform an action (to show/hide a custom field) when users select a category or a post format or a page template. Without ability to target to the category meta box, we can't do that. |
Tran, as I described above that won't work, you can't target React elements
and attach event listeners like that.
As soon as someone selects a term or unselected a term, or even expands and
contracts the panel, your event listeners would need recreating. If the
panel is closed there wouldn't even be DOM nodes to match against. Simply
changing the block you're in would change the sidebar undoing what your
code that targets the Html class did
Instead, you'd need to use the JS APIs available and do it the proper data
driven way, either via the data store or the JS WP Hook system that tries
to recreate actions and filters
…On Wed, 5 Dec 2018 at 09:37, Tran Ngoc Tuan Anh ***@***.***> wrote:
I'd love to add unique classes or IDs.
One reason is for *consistency* as @elliotcondon
<https://github.com/elliotcondon> said. I noticed other issues as well:
Take the *Page Attributes* meta box as an example. The CSS is following
BEM style, but we can't find the class for the *block*, but inside
elements. In details, we have only .editor-page-attributes__template, not
.editor-page-attributes. So it's kind of the missing a parent class.
Another reason is it's nearly impossible to *target a meta box*. For
example, there's no difference between taxonomies meta boxes. We can't
differentiate built-in Category meta box vs. custom taxonomy meta box.
The reason we need to target a meta box is: in our situation, we want to
perform an action (to show/hide a custom field) when users select a
category or a post format or a page template. Without ability to target to
the category meta box, we can't do that.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12082 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADl53jVNntanrk9h5RB1iiLZBRO_KYyks5u15PagaJpZM4YqFc4>
.
|
Hi @tomjn jQuery event listeners can be added to a parent element in a way that allows for changing inner elements like so:
That said, I think we are all happy to use the new JS APIs available. The only problem is that we can't find any resources. Can you please provide a code example on how we can "listen" for a change of a post attribute. For example, if the user selects "Category 2" (taxonomy term), I would like to run some custom JS. How can we do this? Thanks in advance. |
You wouldn't listen to the UI, you would listen for the change to the
internal data structure.
Also don't forget I'm just another commenter, my daily job is code review.
I don't speak in an official capacity so I don't know the ins and outs of
developer docs though I know some are in progress, I don't know their state
I do know that the JS hooks stuff was on the core make blogs and their APIs
is as close to the PHP equivalent as possible, and that the data store is
on GitHub with a tonne of markdown docs, and referenced in the handbook too.
…On Wed, 5 Dec 2018 at 10:08, Elliot Condon ***@***.***> wrote:
Hi @tomjn <https://github.com/tomjn>
jQuery event listeners can be added to a parent element in a way that
allows for changing inner elements like so:
$(document).on('click', '.category-items input', function( e ){
// do something
});
That said, I think we are all happy to use the new JS APIs available. The
only problem is that we can't find any resources.
Can you please provide a code example on how we can "listen" for a change
of a post attribute. For example, if the user selects "Category 2"
(taxonomy term), I would like to run some custom JS. How can we do this?
Thanks in advance.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#12082 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADl5yzk-V6IIbWUDiOg1diYxSyaigIGks5u15sWgaJpZM4YqFc4>
.
|
I agreed with @elliotcondon that we can listen to the That sounds familiar with the hooks in PHP. So, the question is, if we have to do with React, does Gutenberg provides these kinds of hooks so we can listen to? For example, here is the source code for Anyway, listening to |
Describe the bug
Sidebar panels (Categories, Featured Image, etc) are missing unique class names.
It would be very helpful to developers if all sidebar panels contained a unique class name (or id) for CSS/JS targeting.
It looks like some of the panels (post status, revision) offer this already and it would be great to have consistency throughout the markup.
Screenshots
![screen shot 2018-11-20 at 10 20 35 am](https://user-images.githubusercontent.com/2296425/48741144-42900700-ecae-11e8-890a-195aa7468a08.png)
Additional context
The text was updated successfully, but these errors were encountered: