- Issue created by @catch
- Merge request !8635Deprecate the page variable and remove documentation for it. → (Open) created by catch
- Status changed to Needs work
8 months ago 4:35pm 2 July 2024 - First commit to issue fork.
- 🇬🇧United Kingdom longwave UK
Please see https://www.drupal.org/node/3334622 → and 📌 Provide a mechanism for deprecation of variables used in twig templates Fixed for how to deprecate Twig variables.
I realised this wasn't documented in the deprecation policy so also opened 📌 Document how to deprecate Twig variables Active to fix that.
- First commit to issue fork.
- 🇮🇳India ankitv18
ankitv18 → changed the visibility of the branch 3458589-deprecate-variablespage-for to hidden.
- 🇮🇳India Sahana _N
Hi
It looks much better. The patch is applied cleanly and I even searched in the Drupal code base for $variables[‘page]. I did not find it. So for me, it looks good.
RTBC ++
- Status changed to Needs review
5 months ago 4:54am 13 September 2024 - 🇳🇿New Zealand quietone
There are failing tests due to get deprecation messages from this deprecation.
The variable is still in use and it looks like it needs to be set to TRUE or FALSE. Can this really be deprecated or do I just not know something?
- Status changed to Needs work
5 months ago 3:48pm 13 September 2024 - Status changed to Needs review
about 2 months ago 7:24am 7 January 2025 - 🇳🇿New Zealand quietone
The 'page' template variable in node templates duplicates 'view_mode' = full.
But 'page' is set to a boolean, how can that be the same as
'view_mode' = full
? - 🇺🇸United States smustgrave
Seems to cause valid test failures in tests.
- 🇬🇧United Kingdom catch
@quietone it's duplicating in the sense that the $page boolean means the same thing as the full view mode. There is some additional logic in setting it, but that logic should not exist at all, just makes it confusing.
- 🇳🇿New Zealand quietone
There are two failing tests. In both of those the page variable is 'false' and the view_mode is 'full'. So, the $page boolean is not the same thing as the full view mode as stated in #15.
The tests are
- core/modules/taxonomy/tests/src/Functional/Views/TermTranslationViewsTest.php
- core/modules/node/tests/src/Functional/NodeTitleTest.php
- 🇬🇧United Kingdom catch
So, the $page boolean is not the same thing as the full view mode as stated in #15.
Yes it's not identical 100% of the time, it's identical about 95-99% of the time but that's what makes it even more confusing.
- 🇳🇿New Zealand quietone
Right, so the MR is completely wrong. Is this is about changing a confusing name or a need to introduce more variables?
The key 'page' is documented for node and clearly states that is is not the same as full mode. And taxonomy also uses 'page' is a similar, though simpler way. It uses
$variables['page'] = $variables['view_mode'] == 'full' && taxonomy_term_is_page($term);
. Would a change be needed there as well? - First commit to issue fork.
- 🇮🇳India vakulrai
I tried to debugged why : core/modules/taxonomy/tests/src/Functional/Views/TermTranslationViewsTest.php its faling and that is because the method : taxonomy_term_is_page($term) is checking for the route \Drupal::routeMatch()->getRawParameter('taxonomy_term') and the test is runing on a view that has a contextual filter argument as : arg_0 .
So , the above check
$variables['page'] = $variables['view_mode'] == 'full' && taxonomy_term_is_page($term);
will always be false , so some additionla logic i needed to that covers the scenario of view. - 🇳🇿New Zealand quietone
@deepakkm and @vakulrai, did you read comment, #18, which starts with, "the MR is completely wrong"? The work to do here is to decide on the resolution, which is shown in the issue summary as 'unknown'.
- 🇬🇧United Kingdom catch
The history here is that Drupal started of with only hard-coded 'teaser' and 'page' template variables.
At some point, I think in Drupal 7, configurable view modes were introduced. However, the template variables were never updated nor clarified.
We didn't remove 'teaser' until 📌 Deprecate $variables['teaser'] Fixed even though it was 100% a duplicate of the view mode.
'page' is slightly more complicated because the logic for 'page' vs 'full' was never brought fully into sync with each other, so there are subtle differences.
However, I think we should be deprecating $variables['page'] and adjusting any necessary logic to rely on view mode = 'full' - e.g. the separation between 'page' and 'full' is because this has just never been cleaned up properly in 15 years, it is not because there was a well thought out discussion about keeping them similar but subtly different concepts.
If someone had a use case for a further view mode, that has the same configuration as 'full', but avoids any logic around the full mode specifically, that can be achieved by configuring 'full' the same as 'default' and then adding an additional view mode that is not overridden.