- Issue created by @matthieuscarset
- Merge request !7907Add missing dependency to shortcut #3445184 β (Open) created by matthieuscarset
- Status changed to Needs review
7 months ago 7:31pm 3 May 2024 - Status changed to RTBC
7 months ago 7:36pm 3 May 2024 - π¨π¦Canada SKAUGHT
have tested branch. attached mov of profile and enabling of Navigation.
works as expected! - πΊπΈUnited States smustgrave
Will leave it in RTBC but donβt think shortcut should be a hard dependency. A lot of sites donβt use shortcuts. Think navigation should handle if shortcuts isnβt installed.
- Status changed to Needs work
7 months ago 9:26pm 3 May 2024 - π¬π§United Kingdom catch
Navigation module could add the shortcuts block to its own configuration in a hook_modules_installed() and then remove it again in hook_modules_uninstalled(). I think that can probably be done here.
- π¨π¦Canada SKAUGHT
thanks @catch
A good thought around keeping a 'softer dependency' this way. - π¬π§United Kingdom catchmoreover, Shortcuts should install into navigation now that it's in core?
Not yet, but the logic could switch over once navigation is stable.
- πͺπΈSpain plopesc Valladolid
Thank you for your work on this.
The root source of this issue is that in LB managing page, apparently
NavigatioShortcutsBlock::blockAccess()
method is not being invoked.That module checks whether shortcuts module is enabled or not before continuing.
Form this point I think we have some alternatives:
- Try to find why blockAccess() is not being invoked and try to invoke it even for the LB config page
- Add an extra check in NavigatioShortcutsBlock::build() to ensure that Shortcut module is enabled
- Continue adding shortcut as a navigation dependency. If so, I would remove all the code checking if Shortcut module is enabled
From my perspective, I would avoid the 3rd option because that would add an unnecessary dependency for the future.
- π¦πΊAustralia larowlan π¦πΊπ.au GMT+10
Agree 3 feels like the worst option
- πͺπΈSpain plopesc Valladolid
Created π BlockComponentRenderArray::onBuildRender() does not check block access in Preview mode Active
To try to address the LB preview issue.
If that gets in, this one could be closed.
- π¨π¦Canada SKAUGHT
Thanks, i see that aspect too.
π In a Drupal site with low resource use (minimal profile) Navigations 'Create section' is missing key items. Active had just noticed some other similar dependency calls working a bit strangely still.
- π¬π§United Kingdom catch
Add an extra check in NavigatioShortcutsBlock::build() to ensure that Shortcut module is enabled
This is probably a good idea for a quick fix to stop the fatal error. Can then figure out better how optional support from different modules should be added.
A question for me is if a module wants to add itself to the toolbar on install, and not just as a menu link, should it be able to do that? If so, then implementing that in shortcut module would provide an example.
- First commit to issue fork.
- Merge request !7944Issue #3445184: Return an empty render array when shortcuts is not enabled β (Open) created by m4olivei
- Status changed to Needs review
7 months ago 4:15pm 7 May 2024 - Status changed to Needs work
7 months ago 11:42pm 7 May 2024 - πΊπΈUnited States smustgrave
Agree with the test suggestion by @larowlan. See there is already ShortcutsNavigationBlockTest with a very extensive test in there. Wonder if at the very end could just uninstall shortcut and do some simple assertions everything is fine.
- Status changed to Needs review
7 months ago 2:07pm 17 May 2024 - πΊπΈUnited States smustgrave
smustgrave β changed the visibility of the branch 3445184-fatal-error-when to hidden.
- Status changed to Needs work
7 months ago 4:58pm 17 May 2024 - π¨π¦Canada SKAUGHT
After installing Minimal profile, then going to admin/extend to enable Navigation (alone) it is initializing without error. As we did not enable Shortcuts the item is not there (so far as expected!).
-next, enable shortcuts module. notice: no "(star) shortcuts" appears at top of all others.
- clear cache. does not "(star) shortcuts" item to appear.
-next step to try: disable navigation. LEAVE Shortcuts ENABLED!.
- clear caches.
- re-enable navigation. still no "(star) Shortcuts" item.
---
meanwhile: reinitialize env -- install regular standard profile, then enable navigation -- "(star) shortcuts" is working as expected. - Status changed to Needs review
7 months ago 5:25pm 17 May 2024 - πͺπΈSpain plopesc Valladolid
Thank you for your review!
I think the scenario described in your manual test is missing the step to create shortcut links.
When there are no shortcut links to show to the user, the shortcuts navigation block is not rendered.
According to https://git.drupalcode.org/project/drupal/-/blob/11.x/core/profiles/stan.... Standard profile creates some shortcut links by default during installation.
It is possible that this is not happening in minimal, where you need to create some shortcut links by yourself to have access to the shortcuts block.
Could you please repeat your tests and confirm this?
Thank you!
- Status changed to RTBC
7 months ago 5:40pm 17 May 2024 - π¨π¦Canada SKAUGHT
Certainly!
It is true, unless I add an item the main branch item doesn't appear.π Navigation glitch with Shortcuts and Minimal Profile Active have opened for followup.
- Status changed to Fixed
7 months ago 10:43am 18 May 2024 - π¬π§United Kingdom catch
Committed/pushed to 11.x and cherry-picked to 11.0.x/10.4.x/10.3.x, thanks!
Automatically closed - issue fixed for 2 weeks with no activity.