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

Show Debug Toolbar Only For HTML Responses #392

Merged
merged 1 commit into from
Feb 7, 2017
Merged

Show Debug Toolbar Only For HTML Responses #392

merged 1 commit into from
Feb 7, 2017

Conversation

SmolSoftBoi
Copy link
Contributor

Only show the debug toolbar for responses containing HTML, allowing other responses (such as JSON and XML) to show clean.

Only show the debug toolbar for responses containing HTML, allowing other responses (such as JSON and XML) to show clean.

Signed-off-by: Kristian Matthews <[email protected]>
@lonnieezell
Copy link
Member

I'll have to consider this more tonight, but one thing that's been on my mental to-do list is to make the toolbar capable of supporting multiple output formats, especially now that we have formatters in the API namespace. Will probably have to switch some things around to make that happen, though.

@lonnieezell
Copy link
Member

Will merge this for now, since I'm not sure when the refactor will be able to happen.

@lonnieezell lonnieezell merged commit 23c8dec into codeigniter4:develop Feb 7, 2017
@tianhe1986
Copy link
Contributor

@EpicKris @lonnieezell
After this PR, I tried to use debug toolbar with default Home controller, but it didn't show.

I added var_dump($format); in DebugToolbar::after and found it was empty string.

So, may it be better to add default content-type header, at least for view() ?

PS: I use Windows 10 + Apache 2.4.23 + PHP 7.0.10.

@SmolSoftBoi
Copy link
Contributor Author

@lonnieezell Should I update this pull request to account for an empty $format or should there be a default content-type header?

@lonnieezell
Copy link
Member

@EpicKris There should be a default content-type header, and I could have sworn it already did that, but I haven't had a chance to look into it.

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

Successfully merging this pull request may close these issues.

3 participants