Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #197: Removed all instances of compact_link. #227

Closed
wants to merge 1 commit into from
Closed

Issue #197: Removed all instances of compact_link. #227

wants to merge 1 commit into from

Conversation

ronliskey
Copy link
Contributor

Pull request for backdrop/backdrop-issues#197

@quicksketch
Copy link
Member

Niiice, I like this change. It simplifies the administrative interface by removing this odd-ball functionality. This PR needs work however. It looks like this change is causing a bunch of PHP notices:

Exception Notice user.admin.inc 805 theme_user_admin_permissions()
Undefined variable: output

Besides the notices, there is now no way to access admin/compact/on and admin/compact/off. We should remove these menu callbacks and the entries in system_menu() to fully remove that functionality.

@@ -802,7 +802,6 @@ function theme_user_admin_permissions($variables) {
foreach (element_children($form['role_names']) as $rid) {
$header[] = array('data' => drupal_render($form['role_names'][$rid]), 'class' => array('checkbox'));
}
$output = theme('system_compact_link');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this line to

$output = '';

Or set the line below to use = instead of .=, either approach is fine.

@quicksketch
Copy link
Member

It looks like there are still a number of references to the system_admin_compact_mode function which we remove in this patch. Places like block.module reference the function like this:

    if ($compact = system_admin_compact_mode()) {
      $class .= ' compact';
    }

@ronliskey ronliskey closed this Sep 16, 2014
@ronliskey ronliskey deleted the 197-ron branch September 16, 2014 01:13
@quicksketch
Copy link
Member

Hi Ron, did you mean to delete this branch? I hadn't gotten it merged into the main project yet. It looked good to go, I just hadn't merged it yet.

@and1truong
Copy link

The patch is available https://github.com/backdrop/backdrop/pull/227.patch

@quicksketch
Copy link
Member

Thanks @andytruong, I was thinking the same thing. I can apply this manually and push it to 1.x. :)

@ronliskey
Copy link
Contributor Author

I’m trying to get a clean setup so I can add a new commit without having the changes from the previous PR included again. Probably being too wild about it but figured it was easy enough to recreate it all once I figure it out. The problem I’m having is the changed files in my previous PR keep getting added to my current PR.

@quicksketch
Copy link
Member

There we go, got it in there: 0c4bc5d

@ronliskey
Copy link
Contributor Author

Hey, you did it despite my bumbling. :-)

jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Jan 5, 2015
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Jan 6, 2015
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Jan 7, 2015
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Jan 7, 2015
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Jan 7, 2015
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Apr 24, 2015
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Jan 1, 2016
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Jan 1, 2016
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Jan 1, 2016
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Jan 1, 2016
jenlampton pushed a commit to jenlampton/backdrop that referenced this pull request Jan 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants