From @catch's comment in #3438895: Add the new Navigation to core as an Experimental module Top Bar feature flag should be based in a feature flag module instead of config.
This might be a good idea, but it's not what I asked for in #3438895: Add the new Navigation to core as an Experimental module, all I was suggesting was a feature flag module per #3423352: Add an API for feature flags and #3432874: Replace "Expose all fields as blocks to Layout Builder" configuration with a feature flag module - i.e. using the module being installed or not to determine whether the feature is enabled, instead of custom config. That way when the top bar becomes stable, navigation could remove the module enabled check and always have it on, then the feature flag module becomes obsolete and removed - instead of needing to update navigation module's configuration. If the top bar should never be enabled independently of the navigation module once everything is stable, then moving it into and out of a module might be too much extra work compared to just a feature flag module. Apologies if this wasn't clear on the other issue.
Issue fork navigation-3442059
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
plopescBackend part is ready for review. We need now to ensure that Top Bar specific styles are moved to the submodule.
Comment #4
ckrinaWorking on the front-end for this.
I wonder if the should call this module just "top_bar" instead of "navigation_top_bar". Let's go with the standards. We can iterate in the future.Comment #5
ckrinaReady for review.
Note on the front-end: moving only the dropdown inside the Top Bar because it'll have the dependency on the Navigation, and will rely on its design system.
Comment #6
catchThis might be a good idea, but it's not what I asked for in #3438895: Add the new Navigation to core as an Experimental module, all I was suggesting was a feature flag module per #3423352: Add an API for feature flags and #3432874: Replace "Expose all fields as blocks to Layout Builder" configuration with a feature flag module - i.e. using the module being installed or not to determine whether the feature is enabled, instead of custom config. That way when the top bar becomes stable, navigation could remove the module enabled check and always have it on, then the feature flag module becomes obsolete and removed - instead of needing to update navigation module's configuration. If the top bar should never be enabled independently of the navigation module once everything is stable, then moving it into and out of a module might be too much extra work compared to just a feature flag module. Apologies if this wasn't clear on the other issue.
Comment #7
finnsky commentedProbably here we also need to add checking if this topBar module not enabled then render some div with button in main module.
Because now in this MR in mobile we don't have navigation at all.
Comment #8
finnsky commentedComment #10
plopescComment #11
plopescCreated new MR based on the suggestions from #6.
Comment #12
catchhttps://git.drupalcode.org/project/navigation/-/merge_requests/258#note_... is exactly what I meant. Could use review of the help text from someone who's more familiar with the functionality.
Comment #13
ckrina@finnsky I've finally separate the Top Bar from the "Control Bar" because they will need to be different bars anyway. There is no way we will be able to fit the collapse button + the more actions + the save button... So they are 2 different things now.
Comment #15
ckrinaThis has been directly merge in 80f2be3d on the MR to core issue.
Comment #17
ckrinaMerging to keep consistency on the contrib code.