This is a module to get the D7 Admin Menu style and feeling into the new Admin Toolbar. It renders Admin Toolbar into one line, removes images and hover effects from the menu and removes title popups while hovering. This module is aimed to developers who work on desktops instead of mobile. Therefore it has no mobile or vertical feature yet.
https://www.drupal.org/sandbox/pucowanje/2790199
git clone --branch 8.x-1.x https://git.drupal.org/sandbox/pucowanje/2790199.git admin_toolbar_slim
https://www.drupal.org/node/2784891
https://www.drupal.org/node/2624458
https://www.drupal.org/node/2783927
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 2790999-03.patch | 784 bytes | Jeff Burnz |
| #7 | 2790999-02.patch | 800 bytes | Jeff Burnz |
| #7 | 2790999-01.patch | 4.06 KB | Jeff Burnz |
Comments
Comment #2
pucowanje commentedComment #3
PA robot commentedThere are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpsgitdrupalorgsandboxpucowanje2790199git
Fixed the git clone URL in the issue summary for non-maintainer users.
We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)
Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #4
pucowanje commentedThe only thing i can see on pareview.sh is suggestion for a PHPUnit Test. This module does not contain larger amounts of PHP as it is an addition to the existing Admin Toolbar Module.
Comment #5
joachim commented> version: VERSION
That doesn't look right. Does your own module's CSS and JS even need a version?
> if (_admin_toolbar_slim_has_access()) {
I'm wondering whether there's an easier way than checking the permission -- presumably if the user doesn't have permission to see the toolbar, then it won't be in the render array? Could you detect that instead?
Also, the contents of that helper function could be reduced to a one-liner:
so that could just be folded in.
There's a bit of clean-up needed with code style and docs formatting:
- missing spaces after control statements, like 'if'
- comments and docblocks need to be complete sentences and wrap to 80 chars.
- a few spare newlines here and there, eg there should be only 1 after the end of the code, not 2.
- missing @file docblock on the module file
Comment #6
pucowanje commentedThanks allot for your review. I've update the module with cleanup to coding style and comments according to your suggestions.
This is also included in the original Admin Toolbar module. I wondered why this should be needed and its good to know that its not needed.
I've seen this approach in the Adminimal Admin Toolbar module. While its not an excuse to have it, i did not find anything useful within $variables or $page in those alter hooks. I even thought of extending it later with a custom permission to give people the possibility of selectively disabling this module, to be able to give the default Admin Toolbar to their end users if they need vertical or mobile layout. I'll probably never add a vertical feature.
Comment #7
Jeff Burnz commentedGreat module, I'll be using this for sure in every site build.
A few things:
Comment #8
Jeff Burnz commentedUnused context and settings?
Comment #9
hesnvabr commentedI applied this module it looks fine.
Comment #10
Jeff Burnz commentedNo, it does not. It fails Drupal CSS coding standards, includes unused fonts (results in pointless font file downloads), unused JS context and settings and the active styles can be broken resulting in an invisible is-active link.
Comment #11
pucowanje commented@Jeff Burnz: Thanks allot for your help with those patches! Somehow i must have missed to insert the fonts. I've updated the module.
Comment #12
pucowanje commentedThis module is obsolete since Jeff Burnz released a similar module while this module was sticking in review. If anyone wants a slim Toolbar, go take a look at here: https://www.drupal.org/project/toolbar_themes
Comment #13
Jeff Burnz commentedEkk, are you sure they're duplicates? I think you're module is far more simple and does one thing. Theres nothing wrong with tightly focused single purpose modules.
Comment #14
pucowanje commentedWell, it might not actually be a direct duplicate of your module but the result is almost the same. I disliked this space wasting appearance of Admin Toolbar and tried to slim it down. If your module gets support for environment coloring with Environment Indicator module, my module would be even more like a clone of yours. I also think that your module does have more support in the features of Admin Toolbar than my module.
Comment #15
avpaderno