-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[WebProfilerBundle] Profiler View Latest should preserve all the current query parameters #15356
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
@@ -112,6 +112,7 @@ public function panelAction(Request $request, $token) | |||
'request' => $request, | |||
'templates' => $this->getTemplateManager()->getTemplates($profile), | |||
'is_ajax' => $request->isXmlHttpRequest(), | |||
'all_query' => $request->query->all(), |
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.
the Request is available in the template, so passing a new variable is not needed
e7ad735
to
138babb
Compare
👍 |
This needs to be rebased on top of the new profiler. but otherwise, 👍 status: reviewed |
138babb
to
d27f934
Compare
It appears the View Latest button got removed altogether at some point with the new profiler. This rebase re-adds it. Because there's limited space to fit the buttons, I've made the following changes:
These changes allow the buttons to fit on one line, since it looks pretty bad if they're split on multiple lines, but it's still a pretty tight fit. Unfortunately, other changes to the new profiler break the original problem I was trying to resolve (specifically: reloading the logs page with the current filter still selected), but this is still better than what we had before. |
@jbafford the "View Latest" button is displayed in the new profiler. Just tested: |
@javiereguiluz In an earlier 2.8 commit, there was actually two buttons: one to view the list of the last ten, but also a button that was added that showed the current profiler page on the latest request (so that you didn't have to click last 10, click the most recent, and then click back to your current profiler page). The second button got removed at some point when the new profiler UI was added. To be clear, I'm referring to the button added in #14264. |
@jbafford I think we should still get rid of this button ... and add it as a link in the "Symfony Profiler" page heading. "Going back to home" is the behaviour that people expect when clicking on the website heading. |
We need a decision here. I trust @javiereguiluz about these kind of things, but if anyone want to give their 2c, please do so now. |
After rethinking about this, I think I was wrong. Putting this link in the @jbafford please do the following changes in
.btn {
padding: .5em .75em;
}
#sidebar #sidebar-shortcuts .btn + .btn {
margin-left: 5px;
}
#sidebar #sidebar-shortcuts .btn {
padding: .5em .6em;
} This is how it should look: |
* Restore View Latest button * In order to fit into the horizontal space available, shorten the names of the Latest Profiles and View Latest buttons and adjust the buttons' margin and padding.
d27f934
to
dc6ee81
Compare
@javiereguiluz I've rebased against the current 2.8 and made the requested changes. |
👍 thanks @jbafford |
Thank you @jbafford. |
…ve all the current query parameters (jbafford) This PR was merged into the 2.8 branch. Discussion ---------- [WebProfilerBundle] Profiler View Latest should preserve all the current query parameters | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | This improves the View Latest link by preserving all of the current query parameters, which makes it more useful with the Logs panel, for example. Commits ------- dc6ee81 Profiler View Latest should preserve all the current query parameters
This improves the View Latest link by preserving all of the current query parameters, which makes it more useful with the Logs panel, for example.