- Issue created by @dagmar
- πͺπΈSpain ckrina Barcelona
Hi @dagmar, thanks for your help with ideas to integrate the Navigation with other modules.
On our designs explorations for the Navigation we already designed for the Environment indicator, in a way that can co-exist with Workspaces. Here are the designs we agreed on, where the initials of the environment are also used as an indicator. They are really close to your explorations, which is great :) Changing the logo background was discarded because the logo can be replaced and either take the whole space or not work well with the color chosen for one of the environments. Credits for this designs should go to @jponch and @anabarcelona.
Either way, I've added this issue to the Navigation roadmap. Thanks!
- π¦π·Argentina dagmar Argentina
@ckrina thanks good to know :) So the navigation module will allow a color change and the environment indicator will have to interact with this new API? No CSS changes are required from this module if I understand this correctly? Any issue in core to check this?
- π¦π·Argentina dagmar Argentina
Found it. π± [PLAN] New Toolbar Roadmap: Path to Beta & Stable Active
- πͺπΈSpain ckrina Barcelona
So the navigation module will allow a color change and the environment indicator will have to interact with this new API?
I have no idea about the implementation, but I bet nobody has thought about it yet. @plopesc and @m4olivei are the right people to answer this. (@m4olivei was who brought this up to my attention so I'm sure he can help)
- First commit to issue fork.
- πΊπΈUnited States trackleft2
I might be wrong, but I think the functions in the navigation module disallowing all block types other than the ones defined by the navigation module itself are what is standing in the way here.
Example https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/navig...
Blocks might need to get a new property called navigation_safe or something similar.
- πΊπΈUnited States michaellander
Ha @trackleft2, I just came here to mention the same thing. I was able to make a quick proof of concept, but I just prepended a render array in
environment_indicator_preprocess_navigation
in$variables['content']['content']
. I think your solution is better, but I wonder if in the interim we could just prepend the rendered block in preprocess(this would make it always first, right after the logo).Additionally, I was curious if inline styles is still the preferred approach, or if we could consider using CSS variables?
$css_variable = '--env-indicator-fg-color: ' . $active_environment->get('fg_color') . ';'; $css_variable .= '--env-indicator-bg-color: ' . $active_environment->get('bg_color') . ';'; $variables['#attached']['html_head'][] = [ [ '#tag' => 'style', '#value' => ":root { $css_variable }", ], 'env-indicator-css-variables', ];
- πΊπΈUnited States trackleft2
@michaellander thanks, I think we should try to keep all that we can from the existing render array in the new block.
I think we should focus on creating the new custom block and Force it into the list with something like
/** * Implements hook_plugin_filter_block__layout_builder_alter(). * * Curate the blocks available in the Layout Builder "Add Block" UI. */ function environment_indicator_plugin_filter_block_alter(array &$definitions, array $extra) { if (($extra['section_storage'] ?? NULL) instanceof NavigationSectionStorage) { $custom_block_id = 'environment_indicator'; $definitions[$custom_block_id] = [ 'class' => 'Drupal\environment_indicator\Plugin\Block\Navigation', 'provider' => 'environment_indicator', 'admin_label' => t('Environment Indicator'), 'category' => t('Navigation'), ]; } } /** * Implements hook_module_implements_alter(). */ function environment_indicator_module_implements_alter(&$implementations, $hook) { if ($hook === 'plugin_filter_block_alter') { // Move environment_indicator to the end of the list. $group = $implementations['environment_indicator'] ?? false; unset($implementations['environment_indicator']); $implementations['environment_indicator'] = $group; } }
- πΊπΈUnited States michaellander
I like that as an interim solve, and would allow us to pivot in the future if core makes the blocks that are allowed override-able. I'd be happy to push this forward while you're gone.
- πΊπΈUnited States michaellander
I've made decent progress on this(see blue button on bottom left):
@ckrina, in the version you have, there are no child links to actually do the environment switching. Did you have another idea of where that belongs or is my approach on the right track(with some cleanup)?
It's using a lazy builder similar to many other blocks in navigation so that we can respect which environment links to show per the users permissions. I should be able to have a fairly complete version pushed back into the fork by tomorrow.
- πΊπΈUnited States michaellander
I pushed up another version:
I did an initial attempt at the CSS, but my CSS nor UI design is my strength. I did use an approach with CSS variables, and I did not create a new
Handler
class. If we do want to go theHandler
route, maybe we take advantage of service tagging in the early returns to avoid hardcoding every potential Handler check.I also have left out the 'configure' link, but not opposed to adding that back either. I could probably just use feedback and/or design inspiration before going further.
- πΊπΈUnited States michaellander
Updated the css to be a bit closer to concepts:
- Status changed to Needs review
about 2 months ago 3:29pm 17 July 2024 - Status changed to Needs work
about 2 months ago 7:12pm 17 July 2024 - πΊπΈUnited States michaellander
It looks like the code is not correctly handling when the 'toolbar' module is turned off, so this needs more work.
- First commit to issue fork.
- πΊπΈUnited States trackleft2
Great progress on this so far.
I think we should move this functionality into a sub module
Then we can add the dependency on the navigation module in the sub module's info file.
We should also set the minimum Drupal Core requirement in the sub module's info file to 11
We should also be able to drop any code that has been deprecated in previous versions of Drupal.
- πΊπΈUnited States trackleft2
Here is a possible README for the sub module.
CONTENTS OF THIS FILE --------------------- * Introduction * Requirements * Installation * Configuration * Maintainers INTRODUCTION ------------ The Environment Indicator Navigation sub-module enhances the Environment Indicator module by adding a navigation block with lazy loading capabilities to display environment-specific links in the Drupal admin toolbar. This sub-module leverages the new Navigation module, which integrates seamlessly with Layout Builder to provide a user-friendly interface for managing navigation menus in Drupal. * For a full description of the module, visit the project page: https://www.drupal.org/project/environment_indicator REQUIREMENTS ------------ This module requires the following modules: * Environment Indicator (environment_indicator) * Navigation (navigation) INSTALLATION ------------ * Install as you would normally install a contributed Drupal module. Visit: https://www.drupal.org/docs/extending-drupal/installing-modules * Enable the Environment Indicator Navigation sub-module on the Extend page (`/admin/modules`) or using Drush: `drush en environment_indicator_navigation` CONFIGURATION ------------- * To add a new navigation block, go to `/admin/config/user-interface/navigation-block`. * Configure the block settings as needed. MAINTAINERS ----------- * Maintained by Michael Lander (mlander) and the Drupal community.
- Merge request !1Close #3457688 Add environment_indicator_navigation sub module β (Open) created by trackleft2
- πΊπΈUnited States trackleft2
@michaellander I've created a merge request with all of my suggested changes against the 4.x branch.
https://git.drupalcode.org/issue/environment_indicator-3457688/-/merge_r...
Note that my version creates a new sub module called `environment_indicator_navigation`
- πΊπΈUnited States michaellander
With Navigation being in core(since 10.3), I don't think it makes sense to require a submodule. And any hooks required simply just won't fire if navigation doesn't exist.
- πΊπΈUnited States trackleft2
Navigation is marked experimental in core, which I also marked the environment_indicator_navigation submodule.
IMHO, the submodule approach adds a layer of safety and better code organization by isolating experimental features and allowing for controlled testing and rollouts.By moving code into a submodule, we ensure that services are only registered when their corresponding module is enabled, keeping the system modular, organized, and performant.
Additionally, functions in the .module file of the submodule are only executed as part of its lifecycle, provided the submodule is enabled.
This method not only keeps the main module clean and stable but also allows for a flexible and maintainable approach to introducing experimental features to a Drupal site.
- πΊπΈUnited States michaellander
The module has already set precedent for handling other toolbars without submodules. When Navigation is the defacto standard, do you intend to move the
ToolbarHandler
behavior into a submodule as well? I think it's a rather confusing experience that if you are using the older toolbar, you just install Environment Indicator core, but if using the new Drupal core Navigation, then you need the submodule. We could always mimc theToolbarHandler
approach in making it responsible for checking if it's applicable(and just move the lazy rendered in there). The submodule just feels unnecessary. - πΊπΈUnited States bronzehedwick
Thanks for all the work on this!
I applied the patch, and I'm not getting the chosen environment color in the side strip or the environment menu. I dumped the variables and it looks like they're still set to the defaults. Can confirm caches are rebuilt.
- πΊπΈUnited States trackleft2
Whatever you guys decide is fine with me.
- πͺπΈSpain ckrina Barcelona
wow, this is going fast and I haven't been able to comment before. Thanks all!
As mentioned, we're working on a design for this that will have similar styles as Workspaces (as it's a stable blocker to get Navigation into stable). I know the modules are different, but the UI is similar in a lost of ways. We're working on the styles for the "drawer" once opened, but we haven't finished that yet.
I'm sharing the designs here so you can see the direction (not finished designs yet since we still need to change several things, like the buttons), but I'd recommend not merging this until we've been able to finish the designs.
When we finish the designs for Workspaces, it'll be implemented on the Navigation itself and you'll be able to reuse part of the code without implementing it here.
About the screenshots I'm seeing: I'd recommend trimming the text as we're doing for the icons in the Navigation instead of applying a gradient. See https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/navig...
- πΊπΈUnited States michaellander
@ckrina - These look awesome! Appreciate the feedback. Definitely would be great to be able to use the same templates as core.
@bronzehedwick - I think it's because some of my css depends on the toolbar module being enabled(unintentionally).
I'm also using CSS variables for part of it, but in some areas I am using inline styles, which is the existing approach. I think it would be nice to align the approach to be the same (All css variables, or all inline styles). I've just been waiting for feedback and maybe things to catch up a bit!
- πΊπΈUnited States trackleft2
Created a new issue for converting inline CSS to CSS variables π Update module to use CSS variables instead of adding inline CSS. Active