Grant author and editor roles permissions to Navigation module once merged in core

Merge request for this issue: https://git.drupalcode.org/project/drupal/-/merge_requests/8929/diffs

---
Thanks to Code Enigma for sponsoring my time to work on this.

Verified with MR applied and doing a fresh install navigation is working for the users.

Re-ran nightwatch test and it was a random failure.

we should probably remove the toolbar module from the installed modules if we're using navigation.

Toolbar removed.

---
Thanks to Code Enigma for sponsoring my time to work on this.

and we'd need to remove the associated permissions in the different roles

Dammit. There's not such thing as a quick win in Drupal!

Ready for review again.

Observed test failures, go through the logic make changes to test case, however left comment on specific code of change.

Pipeline passed successfully & MR is mergeable now..

@pooja_sharma thanks for working on this. I think instead of removing that test, we should have a clause in it that uninstalls the navigation module and enables the toolbar module instead.

Or else we'll need a follow-up issue to place that message somewhere else and update the test to reflect the new position of it.

@markconroy thanks for the suggestions, uninstalled navigation module, enabled the toolbar module along with added comment as well

PLease review, Keeping it in NR (prev state)

Thanks for your continued work @pooja_sharma, but we can't uninstall navigation and install toolbar. If we do that we are back to the exact same state that Drupal core is currently in. The point of this issue to to install navigation and to stop using toolbar.

I'll revert your commits.

---
Thanks to Code Enigma for sponsoring my time to work on this.

1. Remove toolbar from modules to enable in demo_umami.info.yml
2. Add navigation to modules to enable in demo_umami.info.yml
3. Move "This site is intended for demonstration purposes" message from toobar to page_top
4. Rename 'toolbar-warning' library and classes to 'demo-profile-warning' and classes
5. Remove 'access toolbar' from permissions in testDemonstrationWarningMessage test
6. Update permissions for author and editor

---
Thanks to Code Enigma for sponsoring my time to work on this.

And (hopefully) finally: Updated the CSS so there is less of it, it's easier to read and we have the same CSS for small screens and large screens

Small screen

Large screen

Code is looking good, asked for confirmation for the design/placement of the warning.

Couple of minor things I noticed, and a question.

Tested the current MR doing a fresh install of Umami

Navigation toolbar appears to be installed and toolbar is not

Menu is created, users have the permissions needed.

Tried a few pages but didn't notice an issue.

Code wise don't see any issue.

Seems everyone was saving at once!

Good to see progress on this! But FYI we already have designs for the Umami message integration after putting some thoughts to it and following the design system we're working on, and it would be great that this would go with those. The left sidebar is supposed to hold all the site-wide information shown on the admin UI, so it doesn't make sense to throw it into the header of the site. Where would you show it int the front-end?

I'd recommend keeping the permissions and the new designs into 2 separated issues so one doesn't block the other.

Good to see progress on this! But FYI we already have designs for the Umami message integration 📌 Integrate Umami message Needs review after putting some thoughts to it and following the design system we're working on, and it would be great that this would go with those. The left sidebar is supposed to hold all the site-wide information shown on the admin UI, so it doesn't make sense to throw it into the header of the site. Where would you show it int the front-end?

I'd recommend keeping the permissions and the new designs into 2 separated issues so one doesn't block the other.

I'd recommend keeping the permissions and the new designs into 2 separated issues so one doesn't block the other.

@ckrina in terms of moving this along quickly, how about we merge this MR "as is" so we have the new navigation module and we also have the "demo purposes" notification. And next week I'll start working on the 📌 Integrate Umami message Needs review (which I hadn't realised existed until now, sorry).

Or would you prefer me to revert the commits that move the message? If so, it will mean we have tests involving the toolbar even though we don't have toolbar installed any more.

I prefer the first option - get this merged, then do the follow-up.

Or would you prefer me to revert the commits that move the message?

As mentioned in my message, yes: not merging things that don't follow the designs would be how I would address things. If you want to move quickly I wouldn't do UI changes into this issue that is supposed to focus on "Grant author and editor roles permissions to Navigation module".

If so, it will mean we have tests involving the toolbar even though we don't have toolbar installed any more.

Then, maybe the other issue needs to get in before this one?