Problem/Motivation

Uninstall and then re-install Bartik and you will get mixed up block mapping in theme. This shouldn't happen.

Proposed resolution

Configuration files should be with the object being configured. Move Bartik configuration files from core/profiles/standard/config/install to core/themes/bartik/config/optional. 'config/optional' is chosen rather than 'config/install' because tests may fail due to unmet dependencies.

CommentFileSizeAuthor
#28 interdiff_24-28.txt21.96 KBvsujeetkumar
#28 2642590_28.patch27.7 KBvsujeetkumar
#24 Move_Bartik_block_config_into_the_Bartik_theme-2642590-24.patch5.19 KBandrewmacpherson
#24 interdiff-2642590-8-24.txt965 bytesandrewmacpherson
#8 interdiff-Move_Bartik_block_config_into_the_Bartik_theme-2642590-3-8.txt0 bytesdcrocks
#8 Move_Bartik_block_config_into_the_Bartik_theme-2642590-8.patch5.24 KBdcrocks
#3 Move_Bartik_block_config_into_the_Bartik_theme-2642590-3.patch4.57 KBdcrocks

Comments

dcrocks created an issue. See original summary.

dcrocks’s picture

dcrocks commented
Related issues: +#2632132: Move Claro standard profile block config into the Claro theme, +#2642588: Move Stark block config into the Stark theme
dcrocks’s picture

dcrocks commented
Status: Active » Needs review
StatusFileSize
new Move_Bartik_block_config_into_the_Bartik_theme-2642590-3.patch4.57 KB

This will probably fail the same way as stark did

Status: Needs review » Needs work

The last submitted patch, 3: Move_Bartik_block_config_into_the_Bartik_theme-2642590-3.patch, failed testing.

dcrocks’s picture

dcrocks commented

The best way to fix these errors is to leave the files in the profile/config/install directories so testing doesn't fail and move a copy into the theme's config/install directory. Not pretty but would work. Any thoughts?

emma.maria’s picture

emma.maria
she/her
commented
Version: 8.0.x-dev » 8.2.x-dev
dcrocks’s picture

dcrocks commented

The problem with this patch is that \Drupal\block\Tests\NewDefaultThemeBlocksTest is ignorant of the theme/config/install|optional directories. It reflects the previous model of block creation during the theme install process. It will have to be rewritten to solve this issue.
Beyond that having 2 copies of the same file in 2 different locations presents maintenance problems for git and I don't think that is the right direction.

dcrocks’s picture

dcrocks commented
Status: Needs work » Needs review
StatusFileSize
new Move_Bartik_block_config_into_the_Bartik_theme-2642590-8.patch5.24 KB
new interdiff-Move_Bartik_block_config_into_the_Bartik_theme-2642590-3-8.txt0 bytes

Used a different theme for the failing test. Really don't think it is efficient to use 'live' themes for tests, even if core, unless they are the objects being tested.

duaelfr’s picture

duaelfr
he/him
Primary language French
Location Montpellier, France
commented
Related issues: +#2665384: Admin theme gets wrong block placement during install when default theme contains block config

Linking to the issue about block config files in config/install.

dcrocks’s picture

dcrocks commented
Category: Task » Bug report

Since an uninstall, install cycle on this theme results in incorrect output, this should be in the bug queue, not task.

joginderpc’s picture

joginderpc
Location Mohali
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +bug not replicated

This theme also working fine as expected.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs review

I'm not convinced by this issue. Firstly the config should depend on Bartik so having it in config/install should not be a problem for some of them - but there's a wider issue. These blocks are configured depending on what modules standard installs. This is standard config - not Bartik. If you install Bartik after installing the minimal profile I would expect to do the block placement myself.

Setting back to "needs review" for more opinions.

One thing that is apparent is that when uninstalling a theme we should list the configuration that will be deleted so the user knows this is not a reversible step. At the moment that does not appear to be happening.

dcrocks’s picture

dcrocks commented

Just a couple of points.
These block config files are specifically for the themes they are defined for, not for the install profiles. They are meaningless to the install profiles if their theme is not also activated by the profile. They were previously in the profile's config directory because that was the only place they could be activated during install.

It has always been bad DX for core themes needing a visit to admin for block placement on install/reinstall. Block inheritance is a nice convenience but really is a nuisance when developing new themes or when working with established themes. Bartik, et all, should always work out of the box, especially for those new to drupal. What kind of experience is drupal providing when a theme becomes broken just because it has gone thru an uninstall/install cycle.

As has been pointed out elsewhere, a theme shouldn't be installed if its dependencies aren't met. Using 'config/optional' instead of 'config/install' provides a soft failure instead of a hard one. Also, bartik and stark are too often used in tests when those tests have absolutely no dependence on those specific themes. These constantly evolving themes are being used instead of static test system themes, resulting in completely unnecessary test failures.

dcrocks’s picture

dcrocks commented

Of the 152 files in a standard install of drupal that have references to Bartik, more than 50 are 'Test' files. I would think a significant number of those would be better off using test system themes rather than a volatile theme that may not even exist in future versions of drupal. The test system should not be dependent on bartik, stark, or admin except where their features are under test.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jacine’s picture

jacine
she/her
Location New York
commented
Related issues: +#2938911: Umami theme hardcodes too many assumptions about the install profile and site configuration

IMO, this block config belongs in Bartik.

Referencing another related issue.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

Here's a thought - if we moved the config into the profile's optional configuration then it would be reinstalled with the theme but it would avoid polluting the theme with config that's dependent on an install profile.

This config does not belong in the theme - it is profile specific.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

andrewmacpherson’s picture

andrewmacpherson commented
StatusFileSize
new interdiff-2642590-8-24.txt965 bytes
new Move_Bartik_block_config_into_the_Bartik_theme-2642590-24.patch5.19 KB

Needed a re-roll after:

  1. #2994579: Remove obsolete login blocks
  2. #3046680: Convert NewDefaultThemeBlocksTest into a Kernel test

Status: Needs review » Needs work

The last submitted patch, 24: Move_Bartik_block_config_into_the_Bartik_theme-2642590-24.patch, failed testing. View results

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: -bug not replicated

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

vsujeetkumar’s picture

vsujeetkumar commented
Status: Needs work » Needs review
StatusFileSize
new 2642590_28.patch27.7 KB
new interdiff_24-28.txt21.96 KB

Fixed tests, Please review.

Status: Needs review » Needs work

The last submitted patch, 28: 2642590_28.patch, failed testing. View results

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Project: Drupal core » Bartik
Version: 10.1.x-dev » 1.0.2
Component: Bartik theme » Look and Feel

Bartik has been removed from core. I am moving this to the contrib project.