Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Copy the System folder templates to Classy. Do not copy the following templates.
./core/modules/system/templates/admin-block-content.html.twig
./core/modules/system/templates/admin-block.html.twig
./core/modules/system/templates/admin-page.html.twig
./core/modules/system/templates/system-admin-index.html.twig
./core/modules/system/templates/system-config-form.html.twig
./core/modules/system/templates/system-themes-page.html.twig
./core/modules/system/templates/status-report.html.twig
./core/modules/system/templates/install-page.html.twig
./core/modules/system/templates/maintenance-task-list.html.twig
These templates will be copied without any edits to them. The editing of the templates will happen in child issues.
Comment | File | Size | Author |
---|---|---|---|
#66 | reorgClassy-x.patch | 25.42 KB | mortendk |
#64 | 2349759-reorgClassy-64.patch | 72.84 KB | rteijeiro |
#63 | 2349759-reorgClassy-62.patch | 130.44 KB | rteijeiro |
#61 | 2349759-reorgClassy-61.patch | 950 bytes | rteijeiro |
#54 | reorgClassy-3.patch | 130.44 KB | mortendk |
Comments
Comment #1
mortendk CreditAttribution: mortendk commentedthis is gonna be a pita ...
Comment #3
falkendk CreditAttribution: falkendk commentedCopied files and removed classes
Comment #5
mortendk CreditAttribution: mortendk commentedComment #6
davidhernandezPlease double-check if any removed classes are being used in javascript. It is best to test the affected template using Stark to make sure nothing is broken.
Comment #9
Maninders CreditAttribution: Maninders commentedI worked on the time.html.twig and on vertical-tabs.html.twig file.
Comment #10
Manjit.SinghCopied files and removed classes from these files select.html.twig, status-messages.html.twig, status-report.html.twig
Comment #11
Mukeysh CreditAttribution: Mukeysh as a volunteer and at gai Technologies Pvt Ltd for gai Technologies Pvt Ltd commentedpatch added for
core/modules/system/templates/system-admin-index.html.twig
core/modules/system/templates/system-config-form.html.twig
core/modules/system/templates/system-themes-page.html.twig
core/modules/system/templates/table.html.twig
core/modules/system/templates/tablesort-indicator.html.twig
core/modules/system/templates/task-list.html.twig
core/modules/system/templates/textarea.html.twig
Comment #12
brahmjeet789 CreditAttribution: brahmjeet789 commentedI added
core/theme/classy/templates/install-page.html.twig
core/theme/classy/templates/item-list.html.twig
core/theme/classy/templates/links.html.twig
core/theme/classy/templates/maintenance-page.html.twig
core/theme/classy/templates/mark.html.twig
core/theme/system/classy/menu-local-action.html.twig
and removed classes from the same files in the core/module/system/templates/..
Comment #13
nitishchopra CreditAttribution: nitishchopra commentedpatch
Comment #14
nitishchopra CreditAttribution: nitishchopra commentedpatch added for
core/themes/classy/templates/form-element.html.twig
core/themes/classy/templates/form.html.twig
core/themes/classy/templates/html.html.twig
core/themes/classy/templates/image.html.twig
core/themes/classy/templates/indentation.html.twig
core/themes/classy/templates/input.html.twig
Comment #15
brahmjeet789 CreditAttribution: brahmjeet789 commentedadded new file
Comment #16
Sumit kumar CreditAttribution: Sumit kumar commentedpatch added for
core/modules/system/templates/menu-local-task.html.twig
core/modules/system/templates/menu-local-tasks.html.twig
core/modules/system/templates/menu.html.twig
core/modules/system/templates/page.html.twig
core/modules/system/templates/pager.html.twig
core/modules/system/templates/progress-bar.html.twig
Comment #17
saki007sterPatch added for
core/modules/system/templates/details.html.twig
core/modules/system/templates/dropbutton-wrapper.html.twig
core/modules/system/templates/feed-icon.html.twig
core/modules/system/templates/field-multiple-value-form.html.twig
core/modules/system/templates/field.html.twig
core/modules/system/templates/fieldset.html.twig
core/modules/system/templates/form-element-label.html.twig
Comment #18
lauriiiComment #19
saki007sterUpdated the name of the patch file
Comment #20
Vidushi Mehta CreditAttribution: Vidushi Mehta commentedRemove and add classes from radios.html.twig, region.html.twig, system-admin-index.html.twig
Comment #21
Jitendra verma CreditAttribution: Jitendra verma commentedFile name :
core/modules/system/templates/admin-block-content.html.twig
core/modules/system/templates/admin-block.html.twig
core/modules/system/templates/admin-page.html.twig
core/modules/system/templates/block--system-branding-block.html.twig
core/modules/system/templates/block--system-menu-block.html.twig
Comment #22
Ashish sandil CreditAttribution: Ashish sandil commentedFile name :
core/modules/system/templates/admin-block-content.html.twig
Comment #23
harishpatel86 CreditAttribution: harishpatel86 commentedFiles name :
core/modules/system/templates/breadcrumb.html.twig
core/modules/system/templates/checkboxes.html.twig
core/modules/system/templates/confirm-form.html.twig
core/modules/system/templates/container.html.twig
core/modules/system/templates/datetime-form.html.twig
core/modules/system/templates/datetime-wrapper.html.twig
Comment #24
lauriiiComment #28
harishpatel86 CreditAttribution: harishpatel86 commentedRecorrection of patch no 23.
Files:
core/modules/system/templates/breadcrumb.html.twig
core/modules/system/templates/checkboxes.html.twig
core/modules/system/templates/confirm-form.html.twig
core/modules/system/templates/container.html.twig
core/modules/system/templates/datetime-form.html.twig
core/modules/system/templates/datetime-wrapper.html.twig
Comment #29
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commentedComment #31
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commentedRetrying as test bot failed with message,
Comment #32
davidhernandezLets go ahead and break up this issue. It is too many templates to deal with in one patch. Any ideas for a logical way to do it? If we did it alphabetically, it would be about 15 separate issues.
Comment #35
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented+1 for breaking this issue. Doing the same alphabetically sounds okay to me. Let me know if I can create the meta issues.
Comment #36
davidhernandezIt is fine with me. Make them a child of the phase 2 meta (#2348543: [meta] Consensus Banana Phase 2, transition templates to the starterkit theme) and add this issue as a related issue. Also, make their names consistent, with system in the title, so they are easier to keep track of. Something like "Copy system--A templates to Classy" or whatever.
Comment #37
davidhernandezAlso, if people have time to work on these issues, please try to work on the block and node ones instead. Those are more important. A list of priorities is in the summary of the phase 2 meta.
Comment #38
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commentedDone! List of issues below,
As a change from davidhernandez's suggestion, I have mentioned this issue (#2349759: Copy system templates to Classy) as parent, to have proper hierarchy and #2348543: [meta] Consensus Banana Phase 2, transition templates to the starterkit theme as related issue.
I hope this is fine. If any change needed, I can update issues as appropriate.
Comment #39
davidhernandezThanks sivaji! I'm marking this one postponed just so people know not to work on it. When the child issues are closed we can close it.
Comment #40
Sumit kumar CreditAttribution: Sumit kumar commentedpatch for
core/modules/system/templates/breadcrumb.html.twig
core/modules/system/templates/checkboxes.html.twig
core/modules/system/templates/confirm-form.html.twig
core/modules/system/templates/container.html.twig
core/modules/system/templates/datetime-form.html.twig
core/modules/system/templates/datetime-wrapper.html.twig
Comment #41
davidhernandezThese should be separate patches added to the child issues. You can see the child issues linked in the sidebar.
Comment #42
webchickThis issue could really do with a summary to explain the rationale for why we're doing this. Because at first glance, this seems to add a pretty big maintenance burden. Now, all routine bug fixes such as #2409817: CKEditor toolbar configuration UI missing ending UL need to be made in two places. Further, rather than the current state in e.g. Bartik where only the template files that themers are mostly likely to want to override being there for easy copy/paste/modify, we're copying *all* of the template files there, even relatively silly ones that only 0.000001% of people would ever override like, for example, the very ckeditor-settings-toolbar.html.twig from that issue, drowning out the "99% cases" (e.g. node.html.twig) in the process. And finally, the very nice-on-the-surface "Simply copy/paste/modify the template in Classy" breaks down as soon as you install a contributed module which of course naturally will not be able to put its stuff in themes/classy.
I raised these concerns with davidhernandez in IRC. He said that the rationale for this change is for usability, so themers don't need to learn weird things like that it's actually System module that provides field.html.twig, not Field module. Also, that if we don't copy all of them, we have to decide on a case-by-case which ones we're going to copy, and then also explain that to themer somehow. "In core vs. not in core" is much easier to document than "In core and arbitrarily decided upon by a committee to be important to themers." He also said this plan already has approval from Alex Pott, so I guess that's fair enough.
However, if that's the case, and we really do want to duplicate 100% of core's templates in Classy, then let's please do it as a single big patch and "rip off the band-aid" once (rather than 20 sub-issues), and from the time that's committed on, duplicate changes / add templates /etc. in both places (and publish a change record to this effect). As opposed to the current situation where it's very tricky to tell if a bug fix should be committed in two places or not without manual checking to see if it's one of the lucky templates that's been copied over yet.
Comment #43
alexpottI feel that all the sub issues should be closed and that this issue should be about deciding what should be copied to classy from system. There are templates for the front- and back- end here. I think all the front-end templates should be copied and we should leave the back-end templates where they are. If we can't decide whether something is front or back then we should err on the side of copying. But, for example, I don't think we should be overwhelming front-end developers with things like
system-admin-index.html.twig
Comment #44
RainbowArrayMy feeling for a while has been that classes that are relevant for system UI should stay within core. For example, the admin toolbar and contextual links should still work if you're using a theme based strictly on core and not on classy. I think that argument holds for template files that are geared towards creating back-end UI rather than user-facing templates.
The argument for moving over everything into Classy would probably be that it would make it easier to create an admin theme from scratch, but it's pretty rare that somebody needs to make an admin theme.
I think it's a solid point that putting a ton of templates that few people will likely change makes things more confusing.
I also agree that if there is any question on whether a template is something that is likely to be themed, it's probably better to err on the side of putting it in Classy.
That's my two cents.
Comment #45
mortendk CreditAttribution: mortendk commentedthe only reason its rare to make admin themes in drupal is that its so %#€%" hard
pretty please lets not do the same mistakes again as we have done with drupal before and only make a bit of the templates visible for the themer.
its inconsistence and gonna bite us further down the road.
Comment #46
alexpottre #44 - I think toolbar and contextual should stay classed in core - since there functionality depends on it. But there should be copies in classy. Because these templates affect the front end. Especially contextual - since content creation and editing is a common task for people that only see the front page. The toolbars templates I'm happy for a consensus to be build - not really fussed.
From a quick glance I would not copy views_ui, color, locale, language, simpletest, update, and the admin templates in system. The admin templates in system are (i think) admin-*, system-*, status-*, install-page and maintenance-task-list
Comment #47
alexpottOh and contextual does not have it's own templates so does not need to be part of the discussion afaics.
Comment #48
davidhernandezGoing with Alex's list from https://www.drupal.org/node/2348543#comment-9564823 we would not copy the following:
I'm uploading a patch copy the remaining system templates, and re-organize Classy into subdirectories. The patch should only have renames and copies. There shouldn't be any edits.
Make sure the directory names are correct, with no typos. And make sure the list of templates is correct.
Once this is in, we can still use the alpha list of child issues to de-classify the system templates that need it.
The only thing I am unsure of from the list is not copying status-report, but copying status-messages?
Comment #49
mortendk CreditAttribution: mortendk commentedstatus messages is used on the normal frontend, so it should be there
- im gonna rant later on about the missing templates that are admin templates, but should not be treated differently.
Even the admin need's a little theme love ;)
not copied over:
RTBC it, as its not the end of the world & a huge pita of a bikeshed if were gonna discuss this now, thats for a later issue :)
Comment #50
davidhernandezWe have to fix Bartik's search form block.
{% extends "@classy/block--search-form-block.html.twig" %}
Does the template extension not find things in subdirectories?
Comment #51
Manuel Garcia CreditAttribution: Manuel Garcia commentedThe system folder seems a bit crowded.. could we organize it a bit ? Grouping these two into their own directories would make it easier for people to find them... sorry if this has already been discussed though! ;)
system/form:
system/menu:
Comment #52
davidhernandez@Manuel, we're going to leave it be for now. The simplest thing is to just mimic the module directories already have it. We may be able to revisit it later.
Uploading a new patch. So you have to specify the template subdirectory. I thought we weren't going to need to do that with the inheritance changes? It seems I also can't remove the @classy. I thought we fixed that too?
Comment #53
mortendk CreditAttribution: mortendk commented@manual nope were gonna do that in seperate issues, this needs to be as small as possible
Comment #54
mortendk CreditAttribution: mortendk commentedfixed the issue as well with block--system-menu-block.html.twig that should be extending on classy not system.
we need an issue on this behaviour, as its strange but works.
{% extends "@classy/system/block--system-menu-block.html.twig" %}
Comment #56
rteijeiro CreditAttribution: rteijeiro commentedLooks good! Reviewed admin pages and everything seems to be working well.
Comment #57
Jeff Burnz CreditAttribution: Jeff Burnz commentedShouldn't this work:
{% extends "block--search-form-block.html.twig" %}
assuming block--search-form-block.html.twig is in Classy I would have thought Bartik will just pick it up and use it, since we have the registry discovery system now for templates?Correct me if I am wrong, I just assumed that would work, I'm doing something similar now but a bit different, sorry no time to test as its 11:40pm and I must be shoving off for the day :/
Comment #58
star-szr#57 shouldn't remove RTBC, please see #2387069: {% extends "foo.html.twig" %} in Twig templates does not respect theme inheritance. Removing the namespace results in an infinite loop.
Comment #59
alexpottThe patch just adds new files - so everything is duplicated. The patch should be really small - it should tell us that a load of files are moving and that there are some minor edits.
Comment #60
Jeff Burnz CreditAttribution: Jeff Burnz commented#58 ok, we probably need to document some details around that - so a recursion only occurs if you try to extend the current template? Thanks for pointing this out, cheers!
Comment #61
rteijeiro CreditAttribution: rteijeiro commentedI think it's fixed now.
Comment #62
mortendk CreditAttribution: mortendk commented@ruben no files are moved over ?
Comment #63
rteijeiro CreditAttribution: rteijeiro commentedOps, wrong diff!
Comment #64
rteijeiro CreditAttribution: rteijeiro commentedAaaarghh! What about now?
Comment #66
mortendk CreditAttribution: mortendk commentedfix git issues locally & rerolled -3
Comment #67
lauriiiWe could also change the template extends #2291449: Add Twig template inheritance based on the theme registry, enable adding Twig loaders
Comment #68
mortendk CreditAttribution: mortendk commented@laurii are you about to scope creep on this ;) ?
Comment #69
star-szr@lauriii - check #57/#58, because it's calling its parent template in the registry that is not possible, at least not currently.
Comment #70
rteijeiro CreditAttribution: rteijeiro commentedBack to RTBC. Let's merge this!
Comment #71
alexpottTemplates are not frozen in beta. Committed b76f263 and pushed to 8.0.x. Thanks!
I also added
crowdcg, sivaji@knackforge.com, dernetzjaeger
from the postponed sub issues to the commit credit.Comment #74
brahmjeet789 CreditAttribution: brahmjeet789 as a volunteer and at gai Technologies Pvt Ltd commentedComment #75
Sumit kumar CreditAttribution: Sumit kumar at gai Technologies Pvt Ltd commented