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

feat(classic UI): how to get a minimal barceloneta integration #1592

Merged
merged 11 commits into from
Mar 17, 2024

Conversation

gforcada
Copy link
Member

@gforcada gforcada commented Dec 13, 2023

Issue number

Description

Explain all the steps to get a custom CSS bundle out of barceloneta theme to style the core/backend parts of Plone Classic UI.

Note for reviewers: it's maybe quite rough and simple, so improvements are very welcome, both in content and syntax/style.

I'm not sure if the !!! tip are allowed at all 😓

Explain all the steps to get a custom CSS bundle out of barceloneta theme to style the core/backend parts of Plone Classic UI.
Copy link

netlify bot commented Dec 13, 2023

Deploy Preview for 6-docs-plone-org ready!

Name Link
🔨 Latest commit cc19c64
🔍 Latest deploy log https://app.netlify.com/sites/6-docs-plone-org/deploys/65f6e4dbcdb27000080c42cc
😎 Deploy Preview https://deploy-preview-1592--6-docs-plone-org.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@gforcada
Copy link
Member Author

@stevepiercy I see on the build log that the reference to the bundle registration with [text](#bundle-registration) seems to not be parsed correctly.

What's the right way to reference a section from within a document?

I see that

{doc}`/backend/vocabularies`

Is to reference another document, and

(classic-ui-forms-label)=

is a way to label headers, so I do need to add a label to the header I want to link to, but then the link itself, how does that need to be rendered? 🤔

Sorry for being a newbie here 😓

@gforcada
Copy link
Member Author

I think that I found a way, not sure if it's the proper way though 😅 🤞🏾

@stevepiercy
Copy link
Contributor

@gforcada we have some docs for Cross-references. The The MyST Syntax Guide > Cross-references is comprehensive, although their examples use triple colons instead of triple backticks as code fences (so-named because they usually fence in or surround code blocks).

Your first attempt to create a cross-reference should have worked. It's usually better to use the {ref} reference role for cross-references to work across all files, even across sets of documentation through Intersphinx, as I think it also generates an entry in the General Index.

It also looks like you got confused about Reference roles, of which there are many. {doc} is used only where the target is a file, whereas {ref} is for any arbitrary target. Labels can be placed almost anywhere.

I'll push up a commit with grammar and syntax fixes. I would like someone from the Classic UI Team to review as a content expert, too, as I am a n00b in that department.

stevepiercy
stevepiercy previously approved these changes Dec 14, 2023
Copy link
Contributor

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed English and MyST syntax and grammar, and the content looks helpful, but I know nothing about the content itself. We need a content expert to review and approve.

@jensens
Copy link
Member

jensens commented Dec 14, 2023

I did not test it, but it should work.

But: For the purpose to get a minimal backend only working we already have base.scss.
First: Why don't you use it? Is it too much/ not enough/ a mix?
But if it is, should we not better change it and document how to use it instead of introducing a new approach here?

@yurj
Copy link
Contributor

yurj commented Dec 14, 2023

I did not test it, but it should work.

But: For the purpose to get a minimal backend only working we already have base.scss. First: Why don't you use it? Is it too much/ not enough/ a mix? But if it is, should we not better change it and document how to use it instead of introducing a new approach here?

base do an @import "grid"; but removing it and importing base.scss is a very good idea if we want a theme that almost works when editing/control panel in Plone.

Terminology

backend theme means when in Classic UI we do edit or control panel or administrative editing.

We want our theme from scratch applied when we're logged and just viewing a page. In this case, we've our theme with the plain plone toolbar on the left. In the theme from scratch we don't want (and should not) style everything but only when "body.viewpermission-view, body.viewpermission-none" which means I've the view permission (logged and viewing the object) or none (anonymous).

Toolbar (my view)

To have the Plone toolbar in our theme from scratch, we've to import the toolbar scss in our theme, import some plone variables and import the color mode variables because of BS 5.3 (@gforcada, I'm doing something like @plone/plonetheme-barceloneta-base/scss/root_variables). When we're editing an object or in the control panel, the barceloneta theme is used (can be done using rules.xml or htmlhead viewlets or conditional bundles for example).

So the main points are:

  • how do I add the plone toolbar in my theme? As proposed in this PR
  • what parts should be themed with my theme and what with barceloneta? Use view permissions! We've 3 ways to do it.

Finally, note that when including the toolbar in my theme, I will use my theme colors and then the toolbar can be different if I'm viewing an object or editing it. We can harmonize removing bs-primary from the toolbar.scss and using the plone-primary color. So the toolbar stay always the same. Actually in my theme the header color of the toolbar is different when I'm using the view or none permission to access the view of the object, because it uses my theme primary color. This can be a feature, if we wish.

Note: I would move the Theme with Diazo in the "from scratch part", the only differences is how the theme css and js are injected in the page. From scratch uses a bundle, diazo uses the diazo bundle created by RR in Plone automatically.

Copy link
Contributor

@yurj yurj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part can be semplified but we've to check it does not interfere with the theme.

docs/classic-ui/theming/from-scratch.md Outdated Show resolved Hide resolved
@stevepiercy
Copy link
Contributor

It sounds like we should put the topic of theming in Classic UI on the agenda for the next Classic UI Team meeting, as there is a lot to consider from this discussion. The existing structure probably should be revised.

For the purpose of reviewing this one new section, I am not concerned with where it should go, but only whether it is a method that we should promote as is in the documentation. It sounds like if @yurj's suggestions can be verified, then that would be the preferred method as it avoids a lot of copy-paste.

What do you all think?

@yurj
Copy link
Contributor

yurj commented Dec 21, 2023

barceloneta theme scss anatomy

To help digging in barceloneta, this is a dive in to its scss:

// Plone colors
// here define colors as values, state colors, bs primary color, link&portlet colors
// if not defined before (uses !default)
@import "variables.colors.plone";
// the same but with bs dark theme
@import "variables.colors.dark.plone";

// Plone specific default variables & properties
// here enable/disable things specific to bs, like caret, shadows
@import "variables.properties";
// here enable/disable barceloneta specific, like spacer, navbar&fonts colors.
// Maybe colors could be moved to variables.colors.*.plone?
@import "variables.barceloneta";

// import "base" starts here

// Bootstrap - imported with --load-path=node_modules
// here it uses all the variable set above, css variables will be filled with
// values below, mostly from root_variables & components
@import "bootstrap/scss/bootstrap";

// Plone backend or minimal styles
@import "alerts";
@import "forms";
@import "controlpanels";
@import "login";
@import "toolbar";
// portal-column-content&friends
@import "grid";
// picture (with variants), figcaption, img, text-columns, image-right/left
@import "content_base";
// table listing, plain, invisible-grid
@import "content_tables";

// import "base" ends here

// generate all the values for plone-xxx variables from variables above as css variables
@import "root_variables";

// here the real barceloneta theme
@import "mixins/mixin.portlets.plone";
@import "mixins/mixin.font.plone";


// Plone components
@import "cards";
@import "scaffolding";


// // style.scss
@import "icons";
// search, anon&member tool (maybe should go in toolbar now?), language selector
@import "header";
@import "sitenav";
@import "breadcrumbs";
// main, article, content-core, section, byline, lead image, 
@import "content";
@import "comments";
@import "portlets";
@import "footer";

@import "print";

@import "roboto-webfont";

// hack to import tinymce-formats , this should be documented
@import "../plonetheme/barceloneta/theme/tinymce/tinymce-formats";

@stevepiercy
Copy link
Contributor

barceloneta theme scss anatomy

Are you proposing a new section? If so, would you either please add a code review comment, push a commit with your suggestion to this branch, or create a new pull request? It's not possible to work with discussion comments. Thank you!

@gforcada
Copy link
Member Author

gforcada commented Mar 8, 2024

Sorry for the delays in my answers 😖

Thanks for all the contributions and suggestions, I hope it is good enough to push it, otherwise, suggestions are welcome!

I think the changes are generic and minimalist enough to get it merged and maintainable for the future 🍀

@yurj
Copy link
Contributor

yurj commented Mar 8, 2024

Hi! I forgot to add that to get the full image scale functionality (mostly responsive images, captions, figures) in the theme, you've to:

@import "@plone/plonetheme-barceloneta-base/scss/content_base";

but this apply the responsive class also to portal logo and all other images (I think this should be fixed in Barceloneta @petschki @MrTango (*)). So what I do in Plone 6.0.10.1 from scratch theme scss is this:

#section-text {
  // import barceloneta rules for responsive images and picture variants
  @import "@plone/plonetheme-barceloneta-base/scss/content_base";
}

So i get nice responsive images on richtext content but not on all the page.

(*) the problem is in this rule:

img,
picture {
  max-width: 100%;
  height: auto;
}

which applies also to logos, template images and everywhere.

@gforcada

@yurj
Copy link
Contributor

yurj commented Mar 8, 2024

Sorry for the delays in my answers 😖

Thanks for all the contributions and suggestions, I hope it is good enough to push it, otherwise, suggestions are welcome!

I think the changes are generic and minimalist enough to get it merged and maintainable for the future 🍀

Good!

I think we should test this if everything is clear and correct from scratch. I'm using this approach now but maybe there's something I've forgot or wrong.

@gforcada
Copy link
Member Author

gforcada commented Mar 8, 2024

Oh, I see, well, maybe we can add it as a comment, that either you provide your own responsiveness, or you need to add more barceloneta stuff.

At the end, this documentation is for being able to reuse parts of barceloneta, otherwise we will pull all of it 😓

Copy link
Contributor

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this LGTM.

I would like an approval from someone who has verified that it works, and then it can be merged. @yurj @jensens @petschki @MrTango

@yurj
Copy link
Contributor

yurj commented Mar 11, 2024

It works here for me. @gforcada?

@gforcada
Copy link
Member Author

Looks good to me as well 👍🏾

@stevepiercy stevepiercy merged commit ef89b06 into 6.0 Mar 17, 2024
6 checks passed
@stevepiercy stevepiercy deleted the gforcada-patch-1 branch March 17, 2024 12:48
@MrTango
Copy link
Contributor

MrTango commented Mar 20, 2024

@yurj

@import "@plone/plonetheme-barceloneta-base/scss/content_base";
but this apply the responsive class also to portal logo and all other images (I think this should be fixed in Barceloneta

but here in the comments is not the right place for it, please create an issue for it and maybe a PR to fix it.
It's take's us more time to read this, than for you to fix it right away, when you have a project open ;)

@MrTango
Copy link
Contributor

MrTango commented Mar 20, 2024

@gforcada @stevepiercy I think the current structure, which is a copy of what we have in the trainings, is not very helpful.
For example the CSS/SCSS infos are valid for Theming from scratch as well as for Theming with Diazo.

I think something like this: #1645

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

Successfully merging this pull request may close these issues.

better isolate toolbar css from rest of the theme
5 participants