Skip to content

Fix env toggling, improve error page styling #8713

Merged
merged 1 commit into from Jan 3, 2013

7 participants

@goshakkk
goshakkk commented Jan 3, 2013

@arunagw
Ruby on Rails member
arunagw commented Jan 3, 2013
@carlosantoniodasilva
Ruby on Rails member

Padding and the fix seem good, not sure I like the black header and the background though.

@goshakkk
goshakkk commented Jan 3, 2013

If black header is really that bad and red looks better, let's at least make it a bit brighter red and get rid of the gradient. Also, background may be lighter, closer to #FFF (e.g. #FAFAFA):

And with red header color matching one on guides:

What do you think?

@carlosantoniodasilva
Ruby on Rails member

The background looks better than white. Do you see any problem with the gradient?

@goshakkk
goshakkk commented Jan 3, 2013

Current gradient just doesn't sense right to me. Flat colors feel better in this case.

@carlosantoniodasilva
Ruby on Rails member

Ok, lets wait for some more feedback on that so.

@Pablo-Merino

IMO I prefer the last one @goshakkk posted (http://d.pr/i/OjNr). I prefer a red header, black header and red letters don't look really good.

@guilleiguaran
Ruby on Rails member

👍 about the last one

@carlosantoniodasilva
Ruby on Rails member

Yup, last one seems good. Make sure you squash your commits, thanks!

@guilleiguaran guilleiguaran merged commit f8633f9 into rails:master Jan 3, 2013
@goshakkk goshakkk deleted the goshakkk:better-error-page branch Jan 3, 2013
@acapilleri

@guilleiguaran there are particular reason to use javascript inline in html?
thanks in advance

@goshakkk
goshakkk commented Jan 3, 2013

I believe the error page embeds CSS into the document and inlines JS to be self-contained.

@acapilleri

I know, but I'd like to understand if there are a specific reasons in the error pages....
for what reason js should be self-contained?

@goshakkk
goshakkk commented Jan 3, 2013

Presume everything in the application breaks—even static file server—the error page will still be rendered properly. I think that's why they include CSS and JS in every page.

@acapilleri

Yes but maybe I was not clear I think that the use of

<script> test = function(){....} </script>

as css
could be better, because the html is coming much complex.

@goshakkk
goshakkk commented Jan 3, 2013

Oh, I get what you mean. Wondering about that as well.

cc @guilleiguaran

@guilleiguaran
Ruby on Rails member

Not sure if there is any reason for it, I just leave it as was before of the change the style 😄

@acapilleri

✌️

@goshakkk
goshakkk commented Jan 3, 2013

So can I move js into functions inside <script> tag and make onclick trigger them? Or will be such PR rejected?

@acapilleri

maybe a new PR is better

@goshakkk
goshakkk commented Jan 3, 2013

Yeah, of course I'll put it into a separate PR.

@acapilleri

but maybe it need more feedbacks

@robin850
Ruby on Rails member
robin850 commented Jan 3, 2013

Awesome! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.