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

{{ parent() }} in root/templates/components/dashboard/navbar.html.twig breaks UI #641

Closed
brunomnsilva opened this issue Feb 7, 2017 · 9 comments

Comments

@brunomnsilva
Copy link
Contributor

This is noticeable in smaller screens, e.g., smartphone (used chromium developer tools).
The mentioned file creates a new navbar in the DOM and it does not allow to access the sidemenu options, because it overlaps the toggle buttom previously set in the parent twigs.

Suggested fix is to add a new empty block to @admin/components/dashboard/navbar.html.twig to be extended by root/templates/components/dashboard/navbar.html.twig.

This twig only adds a "LOGIN WITH ROOT ACCOUNT" to the navbar.

@brunomnsilva
Copy link
Contributor Author

Current UI:
prior-fix

After proposed fix:
after-fix

@lcharette
Copy link
Member

The root message should actually be hidden on small width devices.

@brunomnsilva
Copy link
Contributor Author

brunomnsilva commented Feb 7, 2017

Don't see any code that would impose that. Nevertheless, the current twig structure breaks the UI the same.

Proposed fix in admin/components/dashboard/navbar.html.twig:

{% block dashboard_navbar %}
    <div class="navbar-custom-menu">
        <ul class="nav navbar-nav">
          <!-- additional elements (e.g., menus, messages) before the user account drop down -->
          {% block dashboard_navbar_extra %} {% endblock %}
          <!-- User Account: style can be found in dropdown.less -->
          {% include "components/user-card.html.twig" %}
        </ul>
    </div>
{% endblock %}

Proposed fix in root/templates/components/dashboard/navbar.html.twig:

{% extends "@admin/components/dashboard/navbar.html.twig" %}

{% block dashboard_navbar_extra %}
  <li><a href="#">{{ translate("HEADER_MESSAGE_ROOT") | upper }}</a></li>
{% endblock %}

However, the hierarchy does not seem natural because the root component is extending an additional admin sprinkle component.

@lcharette
Copy link
Member

It's not hidden currently, but it should be as a different locale could display a longer string that would be wrapped on two or more line anyway (for example French: VOUS ÊTES CONNECTÉ EN TANT QUE L'UTILISATEUR ROOT).

Can you submit a pull request with the proposed changes?

brunomnsilva added a commit to brunomnsilva/UserFrosting that referenced this issue Feb 7, 2017
@brunomnsilva
Copy link
Contributor Author

Done!

lcharette pushed a commit that referenced this issue Feb 7, 2017
* Fix navbar UI issue #641
* correct conflict with upstream/develop fetch
@lcharette
Copy link
Member

PR Merged !

Like I said, there's still an issue when the locale is set to french as the string is longer and make things warp on more than one line. I'll just hide it on mobile for now.

capture d ecran 2017-02-07 a 16 48 35

@lcharette
Copy link
Member

There we go. Tested on different screen size using Safari Responsive Design Mode ;)

@brunomnsilva
Copy link
Contributor Author

brunomnsilva commented Feb 7, 2017 via email

@lcharette
Copy link
Member

Other situation will require a more sophisticated fix, I agree, but as far as the root message goes, I guess it's enough. That string shouldn't be seen by many users (well, one actually).

For the best user experience on a mobile size, icons should be used anyway

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

No branches or pull requests

2 participants