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

Change markup of fields #364

Open
alessandrotesoro opened this issue Jun 24, 2015 · 43 comments
Open

Change markup of fields #364

alessandrotesoro opened this issue Jun 24, 2015 · 43 comments

Comments

@alessandrotesoro
Copy link

Hello,

I was wondering whether you guys are interested in changing the layout and markup of the metaboxes. I've built a custom metaboxes library myself but it's obviously not up to par with your CMB2. Here you can see a screenshot of how it looks like https://dl.dropboxusercontent.com/spa/fqbd10dq98t1met/m3lsuj7j.png would you be interested in applying this same layout to your library?

If not, i've had a quick look at your code and it seems there isn't a way to change the markup of the fields rendering function, or i've missed it ?

@jtsternberg
Copy link
Member

To change the field markup would be an insane amount of work. (trust me I know, we've done it already to convert from tables). I'd be much more interested in styling the existing markup to look that way.

@alessandrotesoro
Copy link
Author

Thanks for the quick reply, i understand.

I'll start trying adjusting the css and see if i can make it look like that 👍 i'll update here once i've got something.

@jtsternberg
Copy link
Member

nice! 👍

@JiveDig
Copy link

JiveDig commented Jun 24, 2015

Jumping in here as i've recently done 'mock' tables via CSS in CMB2.
frontend-edit-profile

I turned off styling on that front-end form and did some custom CSS.

@jtsternberg
Copy link
Member

That looks nice! That's a good reminder.. though you turned the css on the front-end, CMB2 fields can be used in a variety of places (which you can see by the partials in the sass folder), and any custom styling needs to work in all contexts.

@JiveDig
Copy link

JiveDig commented Jun 24, 2015

On mobile it goes to...
frontend-edit-profile-mobile

btw.. is proper etiquette to link to images? Or just upload them here?

@jtsternberg
Copy link
Member

yah, that's nice. I'm not sure there is 'proper etiquette', but if you upload images here, they are more likely to be around as long as this issue/repo is (vs hot-linking and links expiring).

@alessandrotesoro
Copy link
Author

Ok so, i've been playing with css for a bit, and so far i've achieved this



I still need to work on the repeatable groups layout.

What do you guys think about it, any feedbacks?

If interested in merging the style into the "core", what kind of tests should i run to make sure the layout looks good ?

@JiveDig
Copy link

JiveDig commented Jun 25, 2015

That looks great. Would love to see something like this get fully tested and merged.

What happens if 'show_names' => false vs 'show_names' => true?

@gregrickaby
Copy link
Contributor

Love it. My only suggestion is to make the website field wider. You have a super wide field for oEmbed, but a medium width for regular 'ol URLs...

@coreymcollins
Copy link

Really digging this as well. I love separating the field names with a background color like this. I agree with Greg's comment on the website field.

I do prefer a lighter and italic description personally, but that's just me. I feel like the description text stands out almost as much as the field in these mockups, whereas currently the text is a fainter gray in italics which doesn't stand out quite as much.

@simonwebdesign
Copy link

Looks nice.

@alessandrotesoro
Copy link
Author

I've tested it with 'show_names' => false

And it looked bit bad, so instead of adding more css to adjust that kind of display, i've added a function to the CMB2 php class that adds 2 classes to the metabox container. 1 of the classes is triggered only when 'show_names' is set to false. You can see the change here. Doing it this way allowed me to add css specific to that scenario.

Furthermore i've also restored the color of the descriptions of the fields to the original one.

@gregrickaby I haven't modified the website field or any of the fields themselves. The length of that field is the one that comes by default with CMB2, what i've modified is just the "wrappers" of the fields and labels. If needed i can change that to fullwidth.

Here's how it looks with show_names => false.




Any opinions/feedbacks :D ?

@coreymcollins
Copy link

I think the show_names => false updates look good with just a couple of notes.

Sometimes you've got the description to the side of the field while other fields have the description sitting below. You can see an example of both in your second link. I'd personally say, for consistency's sake, the description should fall in the same spot for every field regardless of input length. I think it makes most sense to have the description fall beneath the content.

I'm also wondering if we should add some default padding to before_row/after_row? In both your first screenshots as well as in the show_names => false screenshots, the text bumps right into the edge. This may be how CMB2 has this setup by default but is something which could potentially be improved upon.

@alessandrotesoro
Copy link
Author

@coreymcollins

I agree with you to put the description beneath the content. However by checking the markup of the description i've noticed a difference between the "inline" description and the ones that fall beneath the field. The inline ones have a "span" tag applied, the other descriptions instead have a "paragraph" tag applied. This leads me to believe that this choice was made with the specific intent of placing certain descriptions inline only.

I'll do some more tests this evening and provide few more screens here later :)

@jtsternberg
Copy link
Member

@alessandrotesoro I've been editing your comments to do so, but can you try to include your images inline so we can preview them here on github? To see how to do it, just edit one of the ones above to see how it's done.

@jtsternberg
Copy link
Member

@alessandrotesoro @coreymcollins the placement of the description is a hold-over from CMB1, even before I got to it. I think it's ok if we change that in core if we think it's better UI.

@JiveDig
Copy link

JiveDig commented Jun 29, 2015

+1 on descriptions underneath on all fields.

@alessandrotesoro
Copy link
Author

I've set the description fields to display underneath the fields


The before_row/after_row haven't been modified since that markup is injected through the hooks.

I've also done a few changes to the repeatable group field - still needs polishing, there's some borders/padding adjustments needed and I need to fix an issue with the opening and closing of the group together with the color of the "X" icon.

Let me know your thoughts on this.

@JiveDig
Copy link

JiveDig commented Jun 29, 2015

I'm curious about times when we don't want a table based layout though. I thought that's what the show_names option was actually.. to show names on the left or on top. It'd be nice to have the option to display labels and fields as block instead of table.

@jtsternberg
Copy link
Member

The main purpose of 'show_names' being set to false if if you want to add a metabox w/ a single field and the metabox name is self-explanatory. An option to show the labels as block instead of table would be cool, but will not be something I'll be personally be adding. So if you're interested, feel free to create a proto-type or pull request.

@jtsternberg
Copy link
Member

@alessandrotesoro I really don't like the two gray colors in the repeatable group example. They look like they have different bases. Can we modify one of those? Maybe even make them the same color? But overall, I am liking the direction

@JiveDig
Copy link

JiveDig commented Jun 30, 2015

Hmmm.. the table vs block thing could be one new parameter when registering the metabox. Something like 'display_table' => true, and it could just add a new class to the metabox, 'cmb-table' or something. Then all the table based CSS can be based off that.

Anybody have thoughts on that idea? I like the table layout for many applications, but sometimes it just doesn't make sense... like metaboxes with minimal fields, or in the sidebar, etc...

@jtsternberg
Copy link
Member

@alessandrotesoro @JiveDig brings up a good point, your styles should definitely take into account being used in the sidebar column. @JiveDig I'm not a fan of adding another metabox parameter to re-enable existing (and expected) behavior, and that would definitely break back-compat, unless it defaulted to true. I guess I'm open to the idea, but to me, the more flexible we make the display/css, the more it will break on edge-cases, like user profile fields, and options pages, sidebars, front-end, etc.

@JiveDig
Copy link

JiveDig commented Jun 30, 2015

@jtsternberg I agree. I don't think the table layout should be default. I think it should default to it's current way... just the fields as block. The new parameter would add table styling (CSS, not td)

@alessandrotesoro
Copy link
Author

As of now this layout i'm working on is only triggered if an additional css class is added to the metaboxes created via CMB2, meaning that the layout is only triggered into post objects pages and nowhere else (not even on frontend). As explained into one of my previous messages, i've accomplished this by adding a little modification to the CMB2 php class, you can see this modification into my commit here https://github.com/alessandrotesoro/CMB2/commit/8d6d622119fd75713b3b84fae68af72d449f500b . This also means that the current CMB2 layout is not affected, i've made no modifications to it. See screenshot here - the cmb2_metabox class is added to the metabox container.

I think a new metabox parameter like "show_table" or "show_as_table" could possibly be set to false by default. I can then modify my function here https://github.com/alessandrotesoro/CMB2/commit/8d6d622119fd75713b3b84fae68af72d449f500b to only trigger the css class when that same parameter is set to true. Thoughts on this?

I've done some more tests on the layout of the repeatable group and made 3 different colors tests, what do you guys think about this ?



Regarding the layout when the metabox is displayed into the sidebar, i will check that once the repeatable group is finished. Possibly, I can see this working the same way as I've explained it above, for example, i could disable the tabled layout when the metabox is in the sidebar, in this way it will use the default cmb2 layout.

Let me know your thoughts 👍

@JiveDig
Copy link

JiveDig commented Jun 30, 2015

I'm liking the direction too... what if the parameter was 'display_layout', and it defaulted to block, but could be set to table as well.. then it would leave room for more options later. Thinking out loud, that could certainly be overkill

@alessandrotesoro
Copy link
Author

Hi guys,

Since i didn't receive any feedback on which repeatable group layout to use, I went ahead and decided to keep the third variation displayed into my previous post. I've also checked the layout within sidebars and made sure it looks good in both scenario, when a metabox is dragged into the sidebar and when a metabox is set to appear into the sidebar by default.

Furthermore i've also added a new parameter to the metabox args that triggers the table layout when set to true. The parameter is called "table_layout". Set it to true to enable the table layout.

If anybody would be kind enough to test it, you can download it here https://github.com/alessandrotesoro/CMB2/archive/master.zip

Let me know your thoughts ;)

@gregrickaby
Copy link
Contributor

Thanks for all your hard work @alessandrotesoro! Will check it out...

@lmartins
Copy link

I also love to see this improved in CMB2, as futile as it might sound, is one my main concerns preventing me from start replacing ACF with CMB2.
So I would also be happy to help with the CSS if any help needed.

@jtsternberg
Copy link
Member

@alessandrotesoro your screenshots disappeared. Any way to get those back?
@lmartins totally understand. ACF is def. pretty. I want CMB2 to have a GREAT UI, but it's also super important to me that CMB2 looks native to WordPress.

@willthemoor
Copy link

Seems to me this ticket should be broken apart. Think the first concern for the CMB2 project itself is to ensure that it's easy to theme. Once that's set, the core library can opt to change it's UI based on contributions and/or users will have an easy way to change it themselves (perhaps via manual CSS include, perhaps via plugin).

I'm not exactly sure what all it would take to get CMB2 into that space but a monster "Let's update the UI ticket" seems like it'll do exactly what this one has done (stay open forever).

Just thinking out loud here (and admittedly under-informed), there's probably a fairly fixed set of form layouts required, most of which can likely be parsed from the contributions in this thread. Identifying those layouts would be a good first step. Adding the ability to pass a layout as a param to a row (or fieldset) would be a good second step! Then, with that param just turning into a wrapping class, it should be easy to adjust the layout with the existing markup via magic flexbox incantations.

Separate from layouts there are type styles and form element styles. Again, if the main metabox container on the page allowed for classes to be passed to it, it would be easy (at least in theory) to apply those types of style adjustments as needed.

I love the "looks native to Wordpress" priority. As long as CMB2 ships with styles having super low specificity, that should be possible while also allowing users an easy way to override them when needed.

@bnecreative
Copy link

I'll add that having an easy to set a classname from new_cmb2_box that would attached to the postbox div would be great. Currently what I've done is filter a custom class using the postbox_classes_{page}_{id} filter. Then I can style the meta box a certain way. I attached a screenshot of my styling as an example.

function bne_cmb_add_metabox_classes( $classes ) {
    
    // Add our classname.
    array_push( $classes, 'bne-cmb-wrapper' );
    
    return $classes;
}
add_filter( 'postbox_classes_page_bne_theme_meta_box_featured_area', 'bne_cmb_add_metabox_classes' );

screenshot160

@jtsternberg
Copy link
Member

@bnecreative does it have to be on the postbox? Putting it on the cmb2-wrap makes it more flexible for CMB2 uses NOT in a metabox.

jtsternberg added a commit that referenced this issue Apr 22, 2016
@bnecreative
Copy link

@jtsternberg, no definitely not. Having it on cmb2-wrap would be perfect as that would also work on a settings page and front end. I only referenced postbox as thats where the filer I was using throws it at.

@jtsternberg
Copy link
Member

As of this commit, it's there. In trunk now.

@bnecreative
Copy link

Awesome. Thanks!

@willthemoor
Copy link

As of this commit, it's there. In trunk now.

Sweet! Thank you.

@jtsternberg
Copy link
Member

jtsternberg commented Apr 25, 2016

Some more thoughts.. it would be great if there was a way to toggle a metabox/group/field from using default cmb2 styles. The way I envision this occurring is that each level (metabox, group, field) would have a specific classname which all others would descend from.

  • Removing the field-specific class from a field (the field row) would cause the cmb2-specific styling for that row to be disabled.
  • Removing the group-specific class from a group field would cause the cmb2-specific group styling to be removed (though the individual field styling would remain).
  • Removing the metabox-specific class from a metabox (or form) would cause the cmb2-specific styling for that metabox/form to be disabled (and all fields/groups/rows within it).

For you front-end masters, does this sound like a good first step?

@gregrickaby
Copy link
Contributor

@jtsternberg From our conversation yesterday, you know that I'm in the middle of trying to create inline repeatable groups for a client. I'd like to get involved here.

I can start breaking out the Sass into proper partials, add Gulp support, then start decoupling global styles/markup as you propose. Would love to see apply_filters around any/all markup. Then, down the road, devs might be able to "theme" CMB2 without getting hacky.

I was setting up some Sass scaffolding this morning, and wanted to show how deep the rabbit hole goes:
cmb2-rabbit-hole

It's quite the "flying v"! 😮

Like @willthemoor said, this process needs to be broken out and will take some time.

@JiveDig
Copy link

JiveDig commented Oct 20, 2016

Is anyone willing/able to start breaking this down int osmaller and more easily managed parts? I'd love to see form styling/layout get leveled up. I know CMB uses WP core styling for the most part, but WP fields, particularly user and post field groups are terribly ugly. I'd love to help, but not sure where to start, besides some of my examples above.

@jtsternberg jtsternberg added this to the CSS Rewrite milestone May 31, 2017
@jtsternberg
Copy link
Member

Bumping... @alessandrotesoro your screenshots disappeared. Any way to get those back?

@jtsternberg
Copy link
Member

For all those following along and interested, I have created a milestone for beginning the work of improving the CSS/UI/markup: https://github.com/CMB2/CMB2/milestone/3

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

No branches or pull requests

9 participants