We have been waiting for a consensus for a while and it is time to move things along.
There a number of issues surrounding layouts, such as the following:
#2956859: Installing Zurb Foundation breaks Manage Layout in Layout Builder
#2871479: Upgrade from layout plugin to Layout API / Layout discovery
#2867688: compatibility problem with drupal-8.3.0 and ds-8.x-3.x
In short: Custom layout classes cannot be declared in themes but can be declared in modules.
The best solution is described in #2871917: Adopt broken layouts from Zurb Foundation theme . ( a workable solution is provided there for the migration too )
All layouts would be removed from the theme and placed in the Foundation Layouts module.
The purpose of this issue to to show that moving layouts to the module is the correct solution and we should implement it.
Agreed?
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | zurb_foundation-remove-layouts-2989534-4.patch | 31.98 KB | sim_1 |
Comments
Comment #2
hongpong commentedHi serg2,
I agree with moving the layouts to the module and have been developing using the patches without problems. I feel like someone told me recently that layouts are allowed in themes when i was at Design4Drupal, but overall this still would be be the best move for maintaining the theme, and it is legitimate since it is an alpha version theme.
Comment #3
serg2 commentedYes, you can put layouts in themes, 100% yes. What you can't do is have custom layout classes in the theme.
See https://www.drupal.org/docs/8/api/layout-api/how-to-register-layouts#using-class-key
(Once we make this move I think we can move to beta!!)
Comment #4
sim_1So it seems like the Foundations Layout module is the best place for this to be handled, rather than try to maintain layouts in two places. I also think if we can simplify the base theme enough to make it more easily maintainable, this theme will have a better chance of keeping up with Drupal 8/9's development.
OK... maybe I'm being a bit heavy-handed, and I understand if this gets tossed out immediately, but I'm submitting a patch that (hopefully) removes the layouts from 8.x-6.x. I think this will move us closer to both goals of handling layouts in the other module, and simplifying what needs to happen in this base theme to keep this theme maintainable.
Comment #5
sim_1I see now that this patch is pretty much a duplicate of the patch on #2867688: compatibility problem with drupal-8.3.0 and ds-8.x-3.x. So I guess my question is if there's a strong reason to wait on getting this in to 8.x-6.x?
Comment #6
hongpong commentedI agree it is the best thing for maintainability and consistency with the rules about layouts to move the layouts to Foundations Layout, but I am wary of posting a breaking change to 8.x-6.x. However we are still in alpha so we are 'allowed' to do so. (I finally closed out a very long project and definitely would like to settle this and, most likely go with your idea)
Comment #7
sim_1This is only a breaking change if people are using the layouts feature in the zurb foundation theme, right? I think when I was last testing this they weren't working, so it seems unlikely that people are using them. But maybe they're using past versions and only more recent versions of the theme have them broken.
Comment #8
hongpong commentedThat's a good point. I pinged the long standing issues that, we should try to finally resolve this this month imo.
I think it is best to move the layouts out into the module, which will simplify maintaining the rest of the theme. If anyone disagrees I hope they can chime in here in the next week or 2.
Comment #9
steveoriol+1 to move the layouts out, it is always better to separate the fact of being able to use the different functions of foundation and examples of uses of it is functionalities that are in fact optional.
It's a bit like the bar of the head, ok she's cool, but personally I have never used it, the customers always have other needs for their top of page. This may also be able to move with layouts ... I say sometime stupid things ;-), but these an idea.
Comment #10
philosurfer commentedI might point out here that in our sub-theme we have removed the layouts.yml and have used the xy_grid_layouts module in place as it has a number of beneficial settings when applying the layout (via third party settings). I suggest any implementation should follow the work being done there.
Comment #11
hongpong commented@philosurfer nice I had not seen that one. Good advice. I added a link to the front page of this project to that!
Comment #12
sim_1@HongPong Circling back to this issue, it seems like the overall agreement is to move forward with removing layouts from the main theme. What are next steps? Does the patch on this branch cover what we'd want to do? Should the patch on the other issue be re-rolled?
Comment #13
sim_1I would like to move forward with this. I think we're in agreement that we can remove layouts. Can someone else test that this patch works for them? In particular, if someone that has an older subtheme, does this break anything? If so, what steps do we need to include in the release notes to help user?
Comment #14
erutan commentedI'd vote on keeping them separate - it allows for a bit more flexibility as there are multiple types of layout in Zurb (or people might just want to do their own using CSS Grids and use the rest of zurb for components etc) as it keeps this side of things nimbler. I used to use Foundation components along with singularity as a grid system for cusstom projetcs as it'd let me have cleaner / more semantic / more maintanable IMO code.
The only headache I'd see coming out of it would be pairing versions of this to the various layout modules, for example the only breaking change for moving from 6.5.3 to 6.6.1 that's of any real impact is the revamping of XY grids... so someone upgrading this would need to pull a new branch of the xy_grids module. That said we're in alpha and things break, and if you put a disclaimer on the project page it seems reasonable to me.
https://github.com/foundation/foundation-sites/releases & https://github.com/foundation/foundation-sites/pull/11405
Comment #15
sim_1Ok, I am moving forward from this. I agree with you @erutan, that we can make notes about the potential breaking impact of this change on the release notes and direct people to pull the latest from the xy_grids module.
I've tested this again and it still applies cleanly to the master branch locally for me.
Comment #17
sim_1Ok I have committed this fix. Yay! thank you to everyone on this thread and the other thread for working through this big change.