-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[WebProfilerBundle] Add collapsed sidebar on small screens #15818
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
Conversation
I'm 👎 on this change too. I purposely removed this feature while the redesign. My reasoning:
@hason in any case, keep in mind that my vote doesn't count for the merging or not of this PR, so you just have to convince the Symfony deciders. |
<ul id="menu-profiler"> | ||
<li> | ||
<a href="#" onclick="javascript: Sfjs.toggleClass(document.body, 'sidebar-collapse'); Sfjs.setPreference('sidebar/collapse', Sfjs.hasClass(document.body, 'sidebar-collapse')); return false;"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
javascript:
in the onclick handler is wrong. You are confusing onclick and special href
I'm not convinced either we need this. I can tell that I never collapsed the profiler menu (I collapse the toolbar sometimes, for instance when it hides some of my own |
👎 for collapsing using javascript, |
The collapsed sidebar is enabled only on small screens. |
739d70a
to
b0bb382
Compare
Now that the behavior of this proposed feature has changed, I change my vote too for 👍 There are very small alignment issues:
|
@javiereguiluz I tried to correct alignment issues, but I'm not a webmaster. Could you review it, please? |
Thank you @hason. |
…eens (hason) This PR was merged into the 2.8 branch. Discussion ---------- [WebProfilerBundle] Add collapsed sidebar on small screens | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Desktop  Mobile   Commits ------- 184d4f2 [WebProfilerBundle] Added collapsed sidebar on small screens
…all screens (hason) This PR was merged into the 2.8 branch. Discussion ---------- [WebProfilerBundle] Add collapsed sidebar on small screens | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | Desktop  Mobile   Commits ------- 184d4f2 [WebProfilerBundle] Added collapsed sidebar on small screens
Desktop

Mobile
