Closed (fixed)
Project:
Escape Admin
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
6 Mar 2014 at 00:17 UTC
Updated:
20 Mar 2014 at 19:21 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
saltednutComment #2
dave reidI'm confused. The 'Back to site' button *should* be hidden by default due to the element-hidden class. It's only if the page is an admin page is that class removed. Why is this necessary?
Comment #3
damienmckennaBecause it still shows:
Comment #4
dave reidHuh, I had some strange Git stuff going on. I re-added what I had to make the element-hidden class override the navbar display:block CSS. http://drupalcode.org/project/escape_admin.git/commit/5094186
Comment #5
saltednutWow, yeah... Just perused the code a bit further. That's a lot of JS, Drupal.settings and CSS tomfoolery to get at what could effectively be solved by changing the #access parameter. OK then.
Comment #6
dave reidThe code is exactly what D8 does. *meh*
Comment #7
dave reidAnd I'm not sure what you're implying here. It's just 4 lines of CSS + JS to make the icon disappear. Everything else is styling the icon on the button, or making sure that the last non-admin page you visited is stored in HTML5 session storage. The purpose is not just to give you a link back to the homepage every time. The purpose is to give you a link back to the page you were on when you started entering the admin side of the site.
Comment #8
saltednutI understand what the function is.
Its 3 lines of PHP to accomplish the same thing if you look at how the patch does it. Just because its how they do it in D8 it doesn't mean they did it correctly.
My main point is: you're doing all this modification of the 'home' button on every single page. Even ones where its not even displayed. Setting array values, changing its function, doing a bunch of stuff. Then drupal bootstraps a non-admin page and renders the home button. Then you call JS settings and hide it with CSS.
Another way to do this - check if its an admin page on the PHP level during the hook that you're already using. If it is, do all the modifications. If its not, just change access to FALSE and be done with it.
Comment #9
dave reidTake it up with the people who added this feature to D8 core (nod_, cha0s, jp.stacey, sun, David_Rothstein according to [#787896). This is an exact backport of that code and will not be anything else. If core changes the way this functionality works, I would backport those changes in a heartbeat.
Comment #10
saltednutI think you should reconsider. Its impossible for it to be an exact backport because D7 and D8 are fundamentally different. Furthermore, the fact that you added this functionality to the D7 toolbar also shows that you are working on a module based on functionality introduced in D8, but that you do not strictly backport the exact functionality. I will be posting a patch that fixes this in D7 and intend to use said patch. Whether or not you can be bothered to consider an improvement to the code of your module is your decision to make.
Comment #11
saltednutFollowup: #2212369: Change Escape Admin to use sessions & Fix Navbar title attribute