8000 Prepare for new toolbar by wouterj · Pull Request #217 · sonata-project/SonataBlockBundle · GitHub
[go: up one dir, main page]
Skip to content

Prepare for new toolbar #217

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 24, 2015
Merged

Prepare for new toolbar #217

merged 1 commit into from
Aug 24, 2015

Conversation

wouterj
Copy link
Contributor
@wouterj wouterj commented Jul 31, 2015

Prepare for the new toolbar design: symfony/symfony#15160

sf-sonata-block-toolbar

I created the SVG from the PNG logo, if you have a proper SVG file, it should be used instead.

@soullivaneuh
Copy link
Member

Is that BC with older toolbar too?

@wouterj
Copy link
Contributor Author
wouterj commented Jul 31, 2015

@soullivaneuh yes, the profiler_markup_version variable defines the version of the toolbar. The older toolbar has no such variable, so it's set to 1 (using the default filter), the upcoming version of the toolbar has version 2.

Using this variable, you can support different versions of the toolbar. E.g. when version 3 comes out, you have:

{% if profiler_markup_version == 1 %}
    {# 2.7- #}
{% elseif profiler_markup_version  == 2 %}
    {# 2.8 #}
{% else %}
    {# latest #}
{% endif %}

@soullivaneuh
Copy link
Member

Okay nice idea. But just major versions?

@soullivaneuh
Copy link
Member

Note: Will review this when symfony/symfony#15160 will be merged.

@soullivaneuh soullivaneuh self-assigned this Jul 31, 2015
@wouterj
Copy link
Contributor Author
wouterj commented Jul 31, 2015

@soullivaneuh toolbar markup version is not bound to Symfony's version. If "3", it just means it's the 3th design of the toolbar (that can already happen in 3.1 for instance).

@soullivaneuh
Copy link
Member

I'm not talking about Symfony version but semantic versioning itself. ;-)

@wouterj
Copy link
Contributor Author
wouterj commented Jul 31, 2015

Ah, in that case: Yes :) (there aren't going to be much redesigns that require markup changes).

@soullivaneuh
Copy link
Member

sonata_block_debug_bar

Two things:

  • Sonata logo seems to be cropped.
  • Maybe we could try to get the same style like other section. Logo + number square. What do you think?

@soullivaneuh
Copy link
Member

And for the second point, maybe we could put a more explicit icon that Sonata logo.

Like this one for example: http://fontawesome.io/icon/cubes/

ping @rande.

@rande
Copy link
Member
rande commented Aug 3, 2015

@wouterj yep you can change the icon as @soullivaneuh suggest.

@soullivaneuh
Copy link
Member

@wouterj SVG format can be found here: https://github.com/FortAwesome/Font-Awesome/blob/master/fonts/fontawesome-webfont.svg

@rande
Copy link
Member
rande commented Aug 5, 2015

ok to test

1 similar comment
@rande
Copy link
Member
rande commented Aug 5, 2015

ok to test

@wouterj wouterj force-pushed the new_toolbar branch 2 times, most recently from 077d8e3 to 7dcfb9d Compare August 9, 2015 15:24
@wouterj
Copy link
Contributor Author
wouterj commented Aug 9, 2015

I like the suggestion of changing the icon to something more describing. I changed it to another icon than the one suggested, as I find the style of this icon more consistent with the other icons and it looks better when it's just 24x24. This is also the icon used on the sonata-project.org website.

Btw, @soullivaneuh, the official sonata project logo is cropped: https://sonata-project.org/bundles/sonatageneral/images/logo.png

@rande
Copy link
Member
rande commented Aug 10, 2015

Ok for the logo

@soullivaneuh
Copy link
Member

Btw, @soullivaneuh, the official sonata project logo is cropped: https://sonata-project.org/bundles/sonatageneral/images/logo.png

Yes, and it's not adapted for the toolbar IMHO.

OK for the logo too, but I would like you to remove "blocks" and setup number rendering as other components:

sonata_block_icons

Like doctrine does.

soullivaneuh
soullivaneuh reviewed Aug 10, 2015
View reviewed changes
Resources/views/Profiler/block.html.twig
<div class="sf-toolbar-block">
{% if profiler_markup_version == 1 %}
<div class="sf-toolbar-icon">
Copy link
Member

Choose a reason for hiding this comment

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

Should be indented.

@wouterj
Copy link
Contributor Author
wouterj commented Aug 23, 2015

Thanks @soullivaneuh, I've fixed the remaining issues.

Please note that the number shown is already equal to the other numbers. The doctrine panel, as shown in each screenshot in this PR, uses the old style. You can see the new styles here: symfony/symfony#15160 I've removed "blocks" though.

soullivaneuh added a commit that referenced this pull request Aug 24, 2015
@soullivaneuh
Merge pull request #217 from WouterJ/new_toolbar
7a3bd55 
Prepare for new toolbar
@soullivaneuh soullivaneuh merged commit 7a3bd55 into sonata-project:master Aug 24, 2015
@soullivaneuh
Copy link
Member

Thank you @wouterj.

@wouterj wouterj deleted the new_toolbar branch August 24, 2015 09:13
@wouterj
Copy link
Contributor Author
wouterj commented Aug 24, 2015

Thanks @soullivaneuh for your review & merge!

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

@soullivaneuh soullivaneuh

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@wouterj @soullivaneuh @rande
0