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

Flesh out twig-template for custom data-collector #5115

Closed
wants to merge 3 commits into from
Closed

Flesh out twig-template for custom data-collector #5115

wants to merge 3 commits into from

Conversation

DHager
Copy link
Contributor

@DHager DHager commented Mar 25, 2015

I think this template will make it easier for people to get started, since it demonstrates a greater range of visual features.

Users less-experienced in twig may have problems with the old skeleton, since it removes the default CSS styles when visiting the main panel. Adding {{parent()}} to the head block makes it friendlier.

@DHager
Copy link
Contributor Author

DHager commented Mar 25, 2015

Side question: Is there specific maximum/ideal pixel-size for profiler icons? The largest ones seem to be around 42x30.

{# the web debug toolbar content #}
{# Used for the menu items along the bottom of the screen. #}
{% set icon %}
<span class="icon"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABoAAAAcCAQAAADVGmdYAAAAAmJLR0QA/4ePzL8AAAAJcEhZcwAACxMAAAsTAQCanBgAAAAHdElNRQffAxkBCDStonIVAAAAGXRFWHRDb21tZW50AENyZWF0ZWQgd2l0aCBHSU1QV4EOFwAAAHpJREFUOMtj3PWfgXRAuqZd/5nIsIdhVBPFmgqIjCuYOrJsYtz1fxuUOYER2TQID8afwIiQ8YIkI4TzCv5D2AgaWSuExJKMIDbA7EEVhQEWXJ6FKUY4D48m7HYU/EcWZ8JlE6qfMELPDcUJuEMPxvYazYTDWRMjOcUyAEswO+VjeQQaAAAAAElFTkSuQmCC" alt=""/></span>
Copy link
Member

Choose a reason for hiding this comment

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

you should recommend using a SVG image IMO, as done in the latest version of Symfony

Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to just add a comment here instead of the image, this will avoid scrollbars. E.g:

{% block toolbar %}
    {# used for the menu items along the bottom of the screen. #}
    {% set icon %}
    <span class="icon">
        <!-- ... your icon image, it is recommended to use an SVG image for best quality -->
    </span>
    {% endset %}

Copy link
Member

Choose a reason for hiding this comment

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

I agree with both of you. I also think that we need to explicitly tell the user that the common practice is to inline icon contents, no matter if it's a base64-encoded PNG or inlined SVG file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stof I think that would be good for the 2.7 branch of the documentation, but for the 2.6-version of each cookbook entry should reflect the 2.6 practice, which as far as I can tell is all PNGs. In other words, let's do that as a separate pull-request.

@wouterj I'd rather keep the actual image, because it provides the user a concrete, working example to build from. Having a scrollbar appear within the black code-sample box is a very small price to pay, especially since most users will probably copy-paste the whole thing and tinker with it outside the web-browser.

@DHager
Copy link
Contributor Author

DHager commented Apr 7, 2015

Is there more than needs to happen to this for it to be ready for 2.6?

For the SVG-icon issue, I'd like to do that as as a separate pull-request against the 2.7 branch.

@DHager
Copy link
Contributor Author

DHager commented Apr 29, 2015

@wouterj , @stof : Is this OK for 2.6? I can make a separate PR against 2.7 using SVG icons.

@DHager
Copy link
Contributor Author

DHager commented May 8, 2015

Ping.

@DHager
Copy link
Contributor Author

DHager commented May 21, 2015

Is there anything I can do to help move this forward?

@xabbuh
Copy link
Member

xabbuh commented May 23, 2015

I think this is good to be merged. Thank you for working on it @DHager.

@weaverryan
Copy link
Member

I think this is just awesome - great work Darien!

weaverryan added a commit that referenced this pull request May 23, 2015
…en Hager)

This PR was submitted for the 2.6 branch but it was merged into the 2.3 branch instead (closes #5115).

Discussion
----------

Flesh out twig-template for custom data-collector

I think this template will make it easier for people to get started, since it demonstrates a greater range of visual features.

Users less-experienced in twig may have problems with the old skeleton, since it removes the default CSS styles when visiting the main panel. Adding `{{parent()}}` to the `head` block makes it friendlier.

Commits
-------

ffaff9b Remove indentations for all-whitespace lines.
b0e2b83 Fix incorrect instructions regarding linking to the "full panel", improve roll-over data example.
cc5ae6a Flesh out twig-template for custom data-collector
@weaverryan
Copy link
Member

Oh, and I agree with what you say about the icons, etc - I think it's a small detail anyways, the real-world example that you have is the most important thing.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants