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

handle bg color at block level poc #329

Closed
wants to merge 20 commits into from
Closed

handle bg color at block level poc #329

wants to merge 20 commits into from

Conversation

danalvrz
Copy link
Contributor

@danalvrz danalvrz commented Jan 18, 2024

@sneridagh this is what I was looking into, I checked it against all pages in https://light-theme.kitconcept.io/vertical-spacing and it is very close to rendering exactly the same. There are a few details pending, the mayor one being that plain text doesn't get BG color assigned (only in edit mode, it currently depends on the blocks-group-wrapper BG), maybe we should change that? As said, it is meant as an exploration. Let me know what do you think.

@sneridagh
Copy link
Member

@danalvrz

First of all, let me note that tackle this the way we want it is extremely complicated, as shown the last years. And coming up with the best solution is complicated. The combinatory out of it is daunting... and we often overlook it. I wept blood tears while trying to develop it, and the block grouping was the result of not willing to deal with that any longer.

So, the point that you are trying to do is that we might not need grouping, and we could spare the block-group-wrapper. If so, then you should remove it in the first place, and make tests without it. Then test in view and edit mode the outcome.

The real challenge here is to write less code to implement what we want, for both edit (and add) and view mode.
The edit mode has its quirks (if you ask me, I would implement it in a far far different way) and it has it's flaws too. But we have to bear with it for now.

The most important question here is: Which block model do you have in mind? We need to adhere to one, that is consistent and the same for all blocks. It can have exceptions, but not every block being one. Think that it's not only you or me touching this CSS or enhance it in projects. The model has to be easy, and understandable, and consistent.

Also, are you aware of all the rules of vertical spacing, right?
In DLR:
https://github.com/kitconcept/dlr-internet/blob/main/docs/development/layout-rules.md

in VLT (subset, but extracted all of them from DLR):
https://github.com/kitconcept/volto-light-theme#vertical-spacing-rules

Also, overlooking margin collapses is wrong, it's in there from the beginning of CSS for a reason. Going all paddings has also its annoyances and edge cases, and it's difficult to teach (since the default is going margins).

Almost all these rules are implemented directly when using a wrapper.

Let's talk about it today!

@sneridagh
Copy link
Member

@danalvrz #330

@sneridagh sneridagh linked an issue Jan 22, 2024 that may be closed by this pull request
danalvrz and others added 19 commits January 24, 2024 16:00
* main:
  Update to Volto 17.12.0 (#342)
  Release 3.0.0-alpha.3
  Fix Fat menu A11y issues (#338)
  Remove extra site-map from header (#340)
  Add new commands to run the image in live mode (#332)
  Upgradetolatest plone and Volto (#331)
  Allow customization the secondary navigation entries via Portal action (#320)
  Fix Introduction-Block inline-styles have wrong typography. (#315)
  Upgrade to Volto 17.11.1 and Plone 6.0.9 (#328)
* main: (21 commits)
  Remove @plone/volto peerDepencency (#364)
  Release 3.1.1
  Fix typo in container deprecation notice (#362)
  Release 3.1.0
  Safer pass by value instead of by reference when modifying internal `blockConfig` data (#361)
  Use the Container in @plone/components (#360)
  Fix spacing between title and description in teaser block.(Breaking change) (#352)
  Fix header tabbing order (#251) (#346)
  Fix show figcaption tag if ther is no caption (#350) (#351)
  Add eu and es translations (#358)
  Release 3.0.1
  Fix teaser styles on add form (#355)
  Missing versions from the last merged PR
  Update to Volto 17.15.1 (#348)
  Fix image gallery opacity. (#347)
  Release 3.0.0
  Add Event metadata block (#290)
  Fix missing key in header (#345)
  Add options to show intranet flag and intranet header. (#318)
  Fix A11y in Navigation (#344)
  ...
@sneridagh
Copy link
Member

sneridagh commented Apr 14, 2024

This has been transferred to the blockModelv3 branch, closing.

#370

@sneridagh sneridagh closed this Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate and experiment on the block model v3
2 participants