Slightly re-design the debugging toolbar. #3833
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Commits ------- 0c9b2d4 use SecurityContextInterface instead of SecurityContext Discussion ---------- [2.0][Security] use SecurityContextInterface instead of SecurityContext see #3522 (this is a fix for the 2.0 branch) --------------------------------------------------------------------------- by pminnieur at 2012-03-21T13:25:59Z *ping* it still missed the 2.0.12 release ... --------------------------------------------------------------------------- by stof at 2012-03-21T16:41:28Z @pminnieur you PR has been merged into master, not into 2.0, so it will only be in 2.1 --------------------------------------------------------------------------- by pminnieur at 2012-03-21T16:43:02Z I know, and this is a second PR for 2.0 branch.
Commits ------- 86a3512 [FrameworkBundle] Add support for full URLs to redirect controller Discussion ---------- [FrameworkBundle] Add support for full URLs to redirect controller I'd consider this a bugfix since at the moment using an URL just redirects to `/current/pathhttp://example.org`.
Commits ------- 15dd17e Simplified CONTENT_ headers retrieval Discussion ---------- Header bag Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes (actually no but I think this has nothing to do with this PR...) Fixes the following tickets: - Todo: -
Commits ------- e4f3fd9 Fixed example code. Discussion ---------- [Translation] example code did not work. The example code for Translation did not compile. And the trans() result was void.
Commits ------- 24a0d0a [DependencyInjection] Support Yaml calls without arguments Discussion ---------- [DependencyInjection] Support Yaml calls without arguments
…lid key child node configuration
Commits ------- fd1ea69 [Security] [HttpDigest] Fixes a configuration error caused by an invalid key child node configuration Discussion ---------- [Security] [HttpDigest] Fixes a configuration error caused by an invalid "key" child node configuration ``` Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes ```
8000
…lid 'key' child node configuration
Commits ------- fc41d4f [Security] [HttpDigest] Fixes a configuration error caused by an invalid 'key' child node configuration Discussion ---------- [Security] [HttpDigest] Fixes a configuration error caused by an invalid "key" child node configuration Bug fix: yes Feature addition: no Backwards compatibility break: no Symfony2 tests pass: 
Commits ------- 595cc11 [Console] Wrap exception messages to the terminal width to avoid ugly output 97f7b29 [Console] Avoid outputing \r's in exception messages Discussion ---------- [Console] Exception rendering fixes This fixes two things: - `\r`'s in exception messages were output (in case of `\r\n` newlines), creating really weird results on windows. - long exception messages were wrapping and then the "red" block was completely messed up, with half black/half red lines, now it's wrapped before output if the terminal width can be detected. If you don't care about merging this for 2.0, you can also merge the `console_ex` branch which applies on master. Due to moving tests and renaming of some normalize stuff in the tests, the two test patches are kind of different. RFC: I am really not sure where to put those getTerminalWidth/Height methods. I guess this is not the best place.
) Commits ------- 04ae7cc [Routing] fixed exception message. f7647f9 [Routing] improved exception message when giving an invalid route name. Discussion ---------- Route collection better exception message Bug fix: no Feature addition: no Backwards compatibility break: no Symfony2 tests pass: yes Fixes the following tickets: - Todo: -
Commits ------- 7ce22f0 [Console] Add docblocks 8a2b115 [Console] Mock terminal size to prevent formatting errors on small terminals Discussion ---------- [Console] Fixes terminal width in tests This fixes the [tests that broke on travis-ci](http://travis-ci.org/#!/symfony/symfony/jobs/1031109) (which seems to advertise a terminal width of ~34, not sure why).
|
you should have rebased @shiroyuki , you are an expert in js telling by your page but here you will have to do things right with git to get this PR merged. I can help you for now just rebase or just rebase a clean branch and perhaps cherry pick your stuff. |
|
It looks like you have worked on the 2.0 branch but submitted the PR on the master one but such changes should be done in the master branch anyway. |
|
@shiroyuki This http://symfony.com/doc/current/contributing/code/patches.html#working-on-a-patch and this might help you: http://symfony.com/doc/current/contributing/code/patches.html#id1 And like @fabpot said, you should do your changes against master branch as they are not bug fixes :) |
|
My mistake... this PR should have been for 2.0. :P Originally it is for 2.1 but until now, i still can't get 2.1 running on my environment. I can rework on this again from master and also clean up my commit too. What do you guys think? (This means this PR will be obsolete until the rework is done... shouldn't be long.) In any case, I forgot to mention that the redesign is available at http://home.shiroyuki.com at the moment. Please let me know if you want me to change something. |
|
@shiroyuki It looks good. I do have to ask you if it's possible to make it so that the memory + execution time panel are visible by default? I mean those are the most important things I look for while using the toolbar. Another cool thing that just crossed my mind would be if we could have the toolbar able to be minimized / maintain its state between requests. What do you guys think? |
|
Great work! Some feedback:
|
|
Nice one, some additional feedback:
|
|
Nice work. I like the opacity feature.. In symfony 1.x we had the ability to reduce the debug bar without closing it but in Symfony2, you just can close it, that's why I think the opacity is a nice improvement. |
|
Opacity is not needed as the toolbar is displayed after all the page content (which was not the case in symfony 1.x where the toolbar was displayed on top of the content). |
|
I experienced some issues where I couldn't see the footers stuck to the bottom of pages. |
|
@willdurand: It's a bug then. |
|
As @willdurand mentioned, the opacity is to solve the problem the problem of the toolbar overlays the footer. The earlier attempt of mine was to use @dlsniper: maintaining the state of toolbar will probably duplicate the functionality of a browser's developer toolbar featuring in Firefox, Safari, Chrome and Opera. Per @stloyd, As I design this with progressive enhancement in mind, most of browsers' specific CSS properties used by the toolbar should have a fallback for incapable browsers. Also, I like the idea that it could be a little bit more descriptive with this design. I will try to incorporate everything else I didn't mention above :D |
|
@fabpot it was the reason why the height of the block behind the toolbar was changed to 80px to allow the toobar to use 2 lines without hiding the content of the page. But you merged a revert a while ago because of people complaining about the extra space when the toobar stayed on 1 line. |
|
but with this new design, the WDT will never be rendered on two lines anymore. |
|
As soon as I can get 2.1 work, I'll close this pull request and send out a new one. :( |
|
I close this pull request for now. But I'm welcome for any idea you might have here. These are the screenshots: |
|
excellent work! |
fabpot
added a commit
that referenced
this pull request
Apr 20, 2012
Commits ------- 75c7d3a Fixed the link to the method with onclick event. ecbabec renamed 'Request handler' to 'Controller' and 'Route ID' to 'Route name'. 3c5ede4 Add a new query to display all information. f6a866b Merged CSS for the toolbar in both embedded mode (on each page) and profiler. 306533b Updated the responsive design in addition to the scenario with authenticated users and exception notification. 4a3312b Updated the toolbar with the responsive design (normal-to-large scenario). 1eec2a2 Updated the toolbar with the responsive design (normal-to-small scenario). 03c8213 Refactored the CSS code for the toolbar out of the template. 37843b3 Updated with PHP logo (only the text). d5e0ccc Made the toolbar to show the version, memory usage, the state of security (both a abbreviation and an associate description) and number of DB requests and request time. 37ad8a6 Removed the check for verbose and adjusted the style when the toolbar is on the top of the page. 67b0532 Redesigned the WDT. Discussion ---------- Re-design the debugging toolbar The toolbar is very useful and containing lots of information. However, as there are too much information, it is very distracting and the toolbar area somehow ends up taking too much space and then becomes something like a panel. The main purpose of this pull request is to hide any information and show only whenever the user wants to see, except the status code and response time. This is based on [the pull request #3833](#3833) with the feedbacks and for 2.1 (master). The testing app is available at http://home.shiroyuki.com. --------------------------------------------------------------------------- by stof at 2012-04-10T06:24:36Z @shiroyuki your testing app denies the access because of the restriction in app_dev.php --------------------------------------------------------------------------- by shiroyuki at 2012-04-10T06:27:27Z @stof: I'm sorry. It should be working now. --------------------------------------------------------------------------- by stof at 2012-04-10T06:45:39Z Moving the toolbar to the top of the page means it will hide some content of the page. You should keep it at the bottom --------------------------------------------------------------------------- by shiroyuki at 2012-04-10T06:48:28Z Just a moment ago, I changed the position of the toolbar via `config_dev` so I could check when WDT is on the top. I just reverted the config file. :D --------------------------------------------------------------------------- by fabpot at 2012-04-10T06:55:16Z Some comments: * I would have kept the number of database request as this number is probably the one everybody should have a look at on every page. * I would have used the original PHP logo (in black and white) instead of a non-standard one But overall, this is a very nice improvement. --------------------------------------------------------------------------- by stloyd at 2012-04-10T06:55:43Z There is an issue with "bubbling" at Firefox 11 (at least), when you hover `<a>` element, the hover event seems to be "launched" twice. --------------------------------------------------------------------------- by fabpot at 2012-04-10T06:56:13Z As the verbose mode has been removed from the template, it should also be removed from the configuration (I can do that after merging if you don't know how to do that). --------------------------------------------------------------------------- by shiroyuki at 2012-04-10T07:05:31Z @stloyd I noticed that too. As I couldn't find the same issue on Webkit-based browsers and all effects on this toolbar heavily relies on CSS, it could have been a glitch on Firefox. @fabpot I'll see what I can do with the number of DB request and the logo. --------------------------------------------------------------------------- by asm89 at 2012-04-10T07:26:28Z Will there be options to somehow keep the debug toolbar 'expanded' or something? I guess the folding of the sf and php information makes sense, but I personally look at the request/time/memory/security and query parts of the toolbar a lot. As my browser window is big enough to show all information at once, this would be a huge step backwards imo. --------------------------------------------------------------------------- by XWB at 2012-04-10T07:28:38Z Agreed with @asm89, I also want the option to show all the information on my screen. --------------------------------------------------------------------------- by fabpot at 2012-04-10T08:28:00Z I tend to agree too with @asm89. What about reusing the `verbose` option for that. This was already its purpose anyway. --------------------------------------------------------------------------- by shiroyuki at 2012-04-10T14:56:45Z How about using media query? --------------------------------------------------------------------------- by shiroyuki at 2012-04-11T02:20:32Z Please note that the latest commit still doesn't have the new logo for PHP. As DoctrineBundle now has its own repository, the change to show the number of DB requests is already done via DoctrineBundle's [PR 57](doctrine/DoctrineBundle#57). --------------------------------------------------------------------------- by guilhermeblanco at 2012-04-11T02:50:47Z @fabpot @shiroyuki as soon as this patch is merged I will do the same on DoctrineBundle. All you need to do is look at me over our desks' separator. =D --------------------------------------------------------------------------- by shiroyuki at 2012-04-11T03:17:41Z The last commit has the updated PHP logo. Unfortunately as @stloyd and @guilhermeblanco pointed out, the flicking on the toolbar when the mouse is over might have been due to the CSS issue on Firefox. --------------------------------------------------------------------------- by Tobion at 2012-04-11T04:46:36Z Nice work shiroyuki. I always had the feeling the toolbar can be improved. Good that you got this one going. I would remove the verbose option (rarely nobody changes it) and use media queries to accomplish a responsive design that shows as much information as possible. And only shows the most important facts when there is not enough space. E.g. the symfony version could be removed if it doesn't fit on the screen because it's mostly static from request to request. --------------------------------------------------------------------------- by Tobion at 2012-04-11T04:48:45Z Another idea: Add a panel "PHP Info" to the profiler that shows the output of `phpinfo()`. This panel is linked from the PHP logo in the WDT which currently has no link on it. --------------------------------------------------------------------------- by shiroyuki at 2012-04-11T15:47:51Z @Tobion: It would be an overkill if `phpinfo()` was visible in the toolbar. Additionally, the toolbar doesn't fit to show that amount of information. Plus, the information released by `phpinfo()` is also static and easily obtained by a simple PHP script. I don't think that WDT should be showing this information. Please note that the media query is not yet implement. The followings are still unknown to me: * should we support the toolbar for mobile device? * what is the minimum screen size? --------------------------------------------------------------------------- by Tobion at 2012-04-11T15:52:43Z @shiroyuki you misunderstood me. phpinfo() should be a new panel in the PROFILER, not the WDT. It is reachable from the WDT by clicking on the PHP logo. But that can be implemented in a seperate PR. It's just an idea and before I would implement it, I'd like to receive feedback if it would be accepted at all. --------------------------------------------------------------------------- by fabpot at 2012-04-11T16:38:44Z Displaying `phpinfo()` data is not in the scope of this PR. --------------------------------------------------------------------------- by Tobion at 2012-04-11T16:48:50Z @fabpot yeah. But would you accept such a PR or do you think it's not useful? --------------------------------------------------------------------------- by fabpot at 2012-04-11T16:57:49Z @Tobion The web profiler is mainly about information for the current request; so I'm not sure it would be useful to have such a tab in the profiler. --------------------------------------------------------------------------- by vicb at 2012-04-11T17:06:15Z @fabpot @Tobion what about adding it in the config panel ? Not sure if it is very useful but I have seen to many `phpinfo.php` in the web root folder. (It could be an expandable panel loaded via ajax like what is used for the Doctrine explain panel). --------------------------------------------------------------------------- by shiroyuki at 2012-04-12T03:11:40Z @tobian @vicb: what kind of information are you looking from `phpinfo()`? --------------------------------------------------------------------------- by Felds at 2012-04-12T03:30:02Z The equivalent for `phpinfo()` was extremely convenient and helped a lot in Symfony 1. It's out of scope but an optional panel could be nice. Ini flags are of great help when debugging on a hurry. :+1: for that! --------------------------------------------------------------------------- by Tobion at 2012-04-12T03:37:52Z @shiroyuki I don't understand your question. Everything of it should be displayed. But don't worry about phpinfo(), I'll work on that in a seperate PR. You can focus on the responsive design. ;) --------------------------------------------------------------------------- by vicb at 2012-04-12T06:54:35Z @shiroyuki I am not looking for anything specific. Just saying I have seen many times customer code using a publicly accessible file to return the info and it would help to get ride of this file. --------------------------------------------------------------------------- by sstok at 2012-04-12T07:59:18Z ``` should we support the toolbar for mobile device? ``` Good question, I don't think so because the screen-size is to small to show anything useful. Maybe a small icon to display the information as overlay, including the token so you can refer to that on a bigger screen?. --------------------------------------------------------------------------- by johnnypeck at 2012-04-13T06:45:43Z If your interested in a useful but not so intrusive way of providing the toolbar on mobile devices perhaps take a look at what the guys at Twitter have done with the topbar navigation converting to a semi-accordion style menu on mobile in Bootstrap. I can see the usefulness. Checkout the responsive.less which makes it easy enough to include/exclude depending on screen size. I found it quite useful in a recent project. Regarding adding a tab for phpinfo, sure it would be useful BUT if the reasoning is that some people leave a publicly available phpinfo script therefore just include it then I would not include it. There are many more useful requirements of the toolbar rather than to insulate intro to web issues. That's like saying don't include the toolbar because someone may build an application that makes the toolbar available publicly (which will happen). I've seen too many projects in my years having no clue of versioning tools that must have been built on the server, live, with filenames like indexv1.php, indexv2.php, indexTryAgain.php, db credentials in the clear, and just hoping to find a point where it works enough. And yes, I've found those scripts were publicly available and still around years after they were created; security holes and all! I'm preaching to the choir here. You'll never stop stupid. All we can do is educate by any means we have and share our knowledge with one another. Aside from that devils advocate reasoning, I would include the phpinfo tab, it does make sense in those random "did I/they compile that in" circumstances. ;-) Sorry for the rant. +1 for mobile sorta+1 for phpinfo +10 for better educating on how to include anything you need so phpinfo could be a "my first foray into adding a tool to my toolbar for Symfony" tutorial in the cookbook. Again, sorry for the long winded rant. Cheers everyone. Goodnight. --------------------------------------------------------------------------- by shiroyuki at 2012-04-13T23:33:21Z @stof I think we can remove the CSS.
gencer
pushed a commit
to gencer/Yaml
that referenced
this pull request
Jul 29, 2014
Commits ------- 75c7d3a Fixed the link to the method with onclick event. ecbabec renamed 'Request handler' to 'Controller' and 'Route ID' to 'Route name'. 3c5ede4 Add a new query to display all information. f6a866b Merged CSS for the toolbar in both embedded mode (on each page) and profiler. 306533b Updated the responsive design in addition to the scenario with authenticated users and exception notification. 4a3312b Updated the toolbar with the responsive design (normal-to-large scenario). 1eec2a2 Updated the toolbar with the responsive design (normal-to-small scenario). 03c8213 Refactored the CSS code for the toolbar out of the template. 37843b3 Updated with PHP logo (only the text). d5e0ccc Made the toolbar to show the version, memory usage, the state of security (both a abbreviation and an associate description) and number of DB requests and request time. 37ad8a6 Removed the check for verbose and adjusted the style when the toolbar is on the top of the page. 67b0532 Redesigned the WDT. Discussion ---------- Re-design the debugging toolbar The toolbar is very useful and containing lots of information. However, as there are too much information, it is very distracting and the toolbar area somehow ends up taking too much space and then becomes something like a panel. The main purpose of this pull request is to hide any information and show only whenever the user wants to see, except the status code and response time. This is based on [the pull request #3833](symfony/symfony#3833) with the feedbacks and for 2.1 (master). The testing app is available at http://home.shiroyuki.com. --------------------------------------------------------------------------- by stof at 2012-04-10T06:24:36Z @shiroyuki your testing app denies the access because of the restriction in app_dev.php --------------------------------------------------------------------------- by shiroyuki at 2012-04-10T06:27:27Z @stof: I'm sorry. It should be working now. --------------------------------------------------------------------------- by stof at 2012-04-10T06:45:39Z Moving the toolbar to the top of the page means it will hide some content of the page. You should keep it at the bottom --------------------------------------------------------------------------- by shiroyuki at 2012-04-10T06:48:28Z Just a moment ago, I changed the position of the toolbar via `config_dev` so I could check when WDT is on the top. I just reverted the config file. :D --------------------------------------------------------------------------- by fabpot at 2012-04-10T06:55:16Z Some comments: * I would have kept the number of database request as this number is probably the one everybody should have a look at on every page. * I would have used the original PHP logo (in black and white) instead of a non-standard one But overall, this is a very nice improvement. --------------------------------------------------------------------------- by stloyd at 2012-04-10T06:55:43Z There is an issue with "bubbling" at Firefox 11 (at least), when you hover `<a>` element, the hover event seems to be "launched" twice. --------------------------------------------------------------------------- by fabpot at 2012-04-10T06:56:13Z As the verbose mode has been removed from the template, it should also be removed from the configuration (I can do that after merging if you don't know how to do that). --------------------------------------------------------------------------- by shiroyuki at 2012-04-10T07:05:31Z @stloyd I noticed that too. As I couldn't find the same issue on Webkit-based browsers and all effects on this toolbar heavily relies on CSS, it could have been a glitch on Firefox. @fabpot I'll see what I can do with the number of DB request and the logo. --------------------------------------------------------------------------- by asm89 at 2012-04-10T07:26:28Z Will there be options to somehow keep the debug toolbar 'expanded' or something? I guess the folding of the sf and php information makes sense, but I personally look at the request/time/memory/security and query parts of the toolbar a lot. As my browser window is big enough to show all information at once, this would be a huge step backwards imo. --------------------------------------------------------------------------- by XWB at 2012-04-10T07:28:38Z Agreed with @asm89, I also want the option to show all the information on my screen. --------------------------------------------------------------------------- by fabpot at 2012-04-10T08:28:00Z I tend to agree too with @asm89. What about reusing the `verbose` option for that. This was already its purpose anyway. --------------------------------------------------------------------------- by shiroyuki at 2012-04-10T14:56:45Z How about using media query? --------------------------------------------------------------------------- by shiroyuki at 2012-04-11T02:20:32Z Please note that the latest commit still doesn't have the new logo for PHP. As DoctrineBundle now has its own repository, the change to show the number of DB requests is already done via DoctrineBundle's [PR 57](doctrine/DoctrineBundle#57). --------------------------------------------------------------------------- by guilhermeblanco at 2012-04-11T02:50:47Z @fabpot @shiroyuki as soon as this patch is merged I will do the same on DoctrineBundle. All you need to do is look at me over our desks' separator. =D --------------------------------------------------------------------------- by shiroyuki at 2012-04-11T03:17:41Z The last commit has the updated PHP logo. Unfortunately as @stloyd and @guilhermeblanco pointed out, the flicking on the toolbar when the mouse is over might have been due to the CSS issue on Firefox. --------------------------------------------------------------------------- by Tobion at 2012-04-11T04:46:36Z Nice work shiroyuki. I always had the feeling the toolbar can be improved. Good that you got this one going. I would remove the verbose option (rarely nobody changes it) and use media queries to accomplish a responsive design that shows as much 90E0 information as possible. And only shows the most important facts when there is not enough space. E.g. the symfony version could be removed if it doesn't fit on the screen because it's mostly static from request to request. --------------------------------------------------------------------------- by Tobion at 2012-04-11T04:48:45Z Another idea: Add a panel "PHP Info" to the profiler that shows the output of `phpinfo()`. This panel is linked from the PHP logo in the WDT which currently has no link on it. --------------------------------------------------------------------------- by shiroyuki at 2012-04-11T15:47:51Z @Tobion: It would be an overkill if `phpinfo()` was visible in the toolbar. Additionally, the toolbar doesn't fit to show that amount of information. Plus, the information released by `phpinfo()` is also static and easily obtained by a simple PHP script. I don't think that WDT should be showing this information. Please note that the media query is not yet implement. The followings are still unknown to me: * should we support the toolbar for mobile device? * what is the minimum screen size? --------------------------------------------------------------------------- by Tobion at 2012-04-11T15:52:43Z @shiroyuki you misunderstood me. phpinfo() should be a new panel in the PROFILER, not the WDT. It is reachable from the WDT by clicking on the PHP logo. But that can be implemented in a seperate PR. It's just an idea and before I would implement it, I'd like to receive feedback if it would be accepted at all. --------------------------------------------------------------------------- by fabpot at 2012-04-11T16:38:44Z Displaying `phpinfo()` data is not in the scope of this PR. --------------------------------------------------------------------------- by Tobion at 2012-04-11T16:48:50Z @fabpot yeah. But would you accept such a PR or do you think it's not useful? --------------------------------------------------------------------------- by fabpot at 2012-04-11T16:57:49Z @Tobion The web profiler is mainly about information for the current request; so I'm not sure it would be useful to have such a tab in the profiler. --------------------------------------------------------------------------- by vicb at 2012-04-11T17:06:15Z @fabpot @Tobion what about adding it in the config panel ? Not sure if it is very useful but I have seen to many `phpinfo.php` in the web root folder. (It could be an expandable panel loaded via ajax like what is used for the Doctrine explain panel). --------------------------------------------------------------------------- by shiroyuki at 2012-04-12T03:11:40Z @tobian @vicb: what kind of information are you looking from `phpinfo()`? --------------------------------------------------------------------------- by Felds at 2012-04-12T03:30:02Z The equivalent for `phpinfo()` was extremely convenient and helped a lot in Symfony 1. It's out of scope but an optional panel could be nice. Ini flags are of great help when debugging on a hurry. :+1: for that! --------------------------------------------------------------------------- by Tobion at 2012-04-12T03:37:52Z @shiroyuki I don't understand your question. Everything of it should be displayed. But don't worry about phpinfo(), I'll work on that in a seperate PR. You can focus on the responsive design. ;) --------------------------------------------------------------------------- by vicb at 2012-04-12T06:54:35Z @shiroyuki I am not looking for anything specific. Just saying I have seen many times customer code using a publicly accessible file to return the info and it would help to get ride of this file. --------------------------------------------------------------------------- by sstok at 2012-04-12T07:59:18Z ``` should we support the toolbar for mobile device? ``` Good question, I don't think so because the screen-size is to small to show anything useful. Maybe a small icon to display the information as overlay, including the token so you can refer to that on a bigger screen?. --------------------------------------------------------------------------- by johnnypeck at 2012-04-13T06:45:43Z If your interested in a useful but not so intrusive way of providing the toolbar on mobile devices perhaps take a look at what the guys at Twitter have done with the topbar navigation converting to a semi-accordion style menu on mobile in Bootstrap. I can see the usefulness. Checkout the responsive.less which makes it easy enough to include/exclude depending on screen size. I found it quite useful in a recent project. Regarding adding a tab for phpinfo, sure it would be useful BUT if the reasoning is that some people leave a publicly available phpinfo script therefore just include it then I would not include it. There are many more useful requirements of the toolbar rather than to insulate intro to web issues. That's like saying don't include the toolbar because someone may build an application that makes the toolbar available publicly (which will happen). I've seen too many projects in my years having no clue of versioning tools that must have been built on the server, live, with filenames like indexv1.php, indexv2.php, indexTryAgain.php, db credentials in the clear, and just hoping to find a point where it works enough. And yes, I've found those scripts were publicly available and still around years after they were created; security holes and all! I'm preaching to the choir here. You'll never stop stupid. All we can do is educate by any means we have and share our knowledge with one another. Aside from that devils advocate reasoning, I would include the phpinfo tab, it does make sense in those random "did I/they compile that in" circumstances. ;-) Sorry for the rant. +1 for mobile sorta+1 for phpinfo +10 for better educating on how to include anything you need so phpinfo could be a "my first foray into adding a tool to my toolbar for Symfony" tutorial in the cookbook. Again, sorry for the long winded rant. Cheers everyone. Goodnight. --------------------------------------------------------------------------- by shiroyuki at 2012-04-13T23:33:21Z @stof I think we can remove the CSS.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The toolbar is very useful and containing lots of information. However, as there are too much information, it is very distracting and the toolbar area somehow ends up taking too much space.
The main purpose of this pull request is to hide any information and show only whenever the user wants to see.
Additionally, the CSS is partially re-organized when we embed the toolbar on each responded page.