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

New skin #57

Closed
wants to merge 21 commits into
base: master
from

Conversation

Projects
None yet
10 participants
@wa0x6e
Contributor

wa0x6e commented Oct 2, 2012

New Skin

There's some majors changes besides the visual aspect:

New priority property

Add $priority property to DebugPanel class. Priority defines the importance of the panel :

  • 0 means 'minor', panel is put on the left side, with a bland title.
  • 1 and greater means 'important', and the panel is on the right side, with a summary title. $priority can take any number, and can be used to order the panels. Important panels are ordered from left to right, from priority 1 to n.

It can also be defined from the controller :

public $components = array(
    'DebugKit.Toolbar' => array(
        'panels' => array('history' => array('priority') => 1)
    )
);

By default, these panels have a priority greater than

  • SqlLog
  • Timer
  • Variables

Use Blocks for displaying panels contents

All panels' content are put in a panelContent block, and are displayed with an echo $this->fetch('panelContent') in debug_toolbar.ctp.

Panel can also override the title, by assigning something to panelTitle block. The original title is available in the panel's element, via $title.

Panel priority is also available inside panel's element with $priority.

Panel title

When pertinent, panels should set the title to something else when priority greater than 0.
This can be done inside the Panel class, in beforeRender(), or inside the panel's elements, by overiding the panelTitle block.

Some panels will always have the same title, regarding of the priority, because there's nothing more to display.
e.g. Request, Session, Environment panels.

Currently, among the panels with priority 0, only the include panel has a different title when its priority greater than 0 (display the number of included files). Promoting other panels will just move the panel to the right side.

Notes

Var panel title doesn't reflect the total number of variables in the views. It does not takes into account the global vars :

  • $title_for_layout
  • $request->data
  • $this->validationErrors
  • Loaded helpers

I have also split the variables list inside the panel into 2 blocks.

Room for improvement

  • Panel title can be set in the Panel class and the panel element. Don't know if it's a good practice.
  • Maybe just display an icon for the panel on the left side.
  • Test new CSS with IE. It works on the last Safari, Firefox and Chrome.

Known issues

  • Panels ordering doesn't keep order for same priority panels
@wa0x6e

This comment has been minimized.

Show comment
Hide comment
@wa0x6e

wa0x6e Sep 14, 2012

Owner

Duplicate of pull via patch-1 branch

Owner

wa0x6e commented on 0b191c8 Sep 14, 2012

Duplicate of pull via patch-1 branch

@yandod

This comment has been minimized.

Show comment
Hide comment
@yandod

yandod commented Oct 3, 2012

👍

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Oct 3, 2012

Member

Looks good, I'll probably merge into a branch until we can get the known issues resolved. I'd like get the panel ordering unified and still keep the additional title content. Two sets of panels feels a bit awkward to me.

Member

markstory commented Oct 3, 2012

Looks good, I'll probably merge into a branch until we can get the known issues resolved. I'd like get the panel ordering unified and still keep the additional title content. Two sets of panels feels a bit awkward to me.

@wa0x6e

This comment has been minimized.

Show comment
Hide comment
@wa0x6e

wa0x6e Oct 3, 2012

Contributor

Panel order issue fixed.

Panels are ordered by priority. Same priority panels keep their order, first declared first displayed.

To resolve the css float right issue that was displaying the panels in the reverse order, I split the

    into 2 lists, one for the minor panels (left), one for major panels (right).

    Finally, maybe we could increase the left panels title to 12 px ?

Contributor

wa0x6e commented Oct 3, 2012

Panel order issue fixed.

Panels are ordered by priority. Same priority panels keep their order, first declared first displayed.

To resolve the css float right issue that was displaying the panels in the reverse order, I split the

    into 2 lists, one for the minor panels (left), one for major panels (right).

    Finally, maybe we could increase the left panels title to 12 px ?

@angelxmoreno

This comment has been minimized.

Show comment
Hide comment
@angelxmoreno

angelxmoreno Sep 3, 2013

Contributor

👍

Contributor

angelxmoreno commented Sep 3, 2013

👍

@elboletaire

This comment has been minimized.

Show comment
Hide comment
@elboletaire

elboletaire commented Oct 5, 2013

👍

@renan

This comment has been minimized.

Show comment
Hide comment
@renan

renan Oct 5, 2013

Member

Lets rebase this baby and merge it in!

@Kamisama Wanna some help rebasing it? This is some good tutorial https://help.github.com/articles/interactive-rebase

Member

renan commented Oct 5, 2013

Lets rebase this baby and merge it in!

@Kamisama Wanna some help rebasing it? This is some good tutorial https://help.github.com/articles/interactive-rebase

@wa0x6e

This comment has been minimized.

Show comment
Hide comment
@wa0x6e

wa0x6e Oct 11, 2013

Contributor

Yes, if someone can help rebasing it, it'll be helpful

Contributor

wa0x6e commented Oct 11, 2013

Yes, if someone can help rebasing it, it'll be helpful

@appinteractive

This comment has been minimized.

Show comment
Hide comment
@appinteractive

appinteractive Oct 12, 2013

Contributor

👍 nice one!

Contributor

appinteractive commented Oct 12, 2013

👍 nice one!

@zeroasterisk

This comment has been minimized.

Show comment
Hide comment
@zeroasterisk

zeroasterisk Jul 3, 2014

Contributor

👍 agreed - it is a better base level...

Contributor

zeroasterisk commented Jul 3, 2014

👍 agreed - it is a better base level...

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Jul 4, 2014

Member

Does anyone have time to rebase rhe changes? I have been trying to focusnon CakePHP 3.0, and will not likely get to this for a few more months.

Member

markstory commented Jul 4, 2014

Does anyone have time to rebase rhe changes? I have been trying to focusnon CakePHP 3.0, and will not likely get to this for a few more months.

@bcrowe

This comment has been minimized.

Show comment
Hide comment
@bcrowe
Member

bcrowe commented Jul 4, 2014

@markstory Yup.

@bcrowe bcrowe referenced this pull request Jul 6, 2014

Closed

New skin #180

@markstory

This comment has been minimized.

Show comment
Hide comment
@markstory

markstory Sep 1, 2014

Member

Closing as I've implemented something similar to this new skin for 3.0.

Member

markstory commented Sep 1, 2014

Closing as I've implemented something similar to this new skin for 3.0.

@markstory markstory closed this Sep 1, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment