Render the console inside the body tag #162
Merged
Conversation
This file contains 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
|
@gsamokovarov: no appropriate reviewer found, use r? to override |
| # possible. | ||
| # | ||
| # The object quacks like Rack::Response. | ||
| class Response < Struct.new(:body, :status, :headers) | ||
| # Returns whether the response is from an acceptable content type. |
Yeah, I will do. |
gsamokovarov
force-pushed
the
content-inside-of-body
branch
from
6401a95
to
1314979
Compare
|
@sh19910711 I've updated the PR. |
| assert_select 'body > #console' | ||
| end | ||
|
|
||
| test '#acceptable_content_type? is truthy if response format is HTML' do |
There was a problem hiding this comment.
This test case seems to be doing the same thing as the above one (test 'render console inside the body tag'). We can merge them into one, perhaps.
|
The code seems good to me.
I like that 👍 |
gsamokovarov
force-pushed
the
content-inside-of-body
branch
from
1314979
to
c8f87c6
Compare
Before this commit, the console was rendered straight after the closing </html> tag, which made the page HTML invalid. However, browsers are used to be molested and they are really good at displaying whatever you throw at them, so that wasn't big problem anyway. Still it was something that bug me quite a bit. I'm trying to redeem myself here. Because I had to change the newly introduce `WebConsole::Response` class I had cleaned the workspace a bit: * `WebConsole::Response` no longer depends on a logger. The detection should be better now, so we don't need to couple the query check in it. * `WebConsole::Response` does one thing only. It used to check the acceptance of a given `Content-Type`. I moved this to the middleware class. * I have reverted the `WebConsole::WhinyRequest` to keep the logging out of the logic in the request class. We no longer need to log in two separate classes, so no need to break `Liskov` or introduce logging into simple query methods.
gsamokovarov
force-pushed
the
content-inside-of-body
branch
from
c8f87c6
to
a7b28cc
Compare
gsamokovarov
added a commit
that referenced
this pull request
Sep 9, 2015
Render the console inside the body tag
Closed
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.
Before this commit, the console was rendered straight after the closing
tag, which made the page HTML invalid. However, browsers are used to be molested and they are really good at displaying whatever you throw at them, so that wasn't big problem anyway. Still it was something that bug me quite a bit. I'm trying to redeem myself here.Because I had to change the newly introduced
WebConsole::ResponseclassI had cleaned the workspace a bit:
WebConsole::Responseno longer depends on a logger. The detectionshould be better now, so we don't need to couple the query check in
it.
WebConsole::Responsedoes one thing only. It used to check theacceptance of a given
Content-Type. I moved this to the middlewareclass.
WebConsole::WhinyRequestto keep the logging outof the logic in the request class. We no longer need to log in two
separate classes, so no need to break
Liskovor introduce logginginto simple query methods.
@sh19910711 do you wanna review this?