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.

Sign up for GitHub

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

Jump to bottom

New 404, 422, 500 pages, they are more stylish and bring ruby essence… #9505

Closed
wants to merge 8 commits into from
Closed

New 404, 422, 500 pages, they are more stylish and bring ruby essence… #9505

wants to merge 8 commits into from

Conversation

Jacke
Copy link

@Jacke Jacke commented Mar 1, 2013

Preview
With new Rails 4 must be a new Service pages
new404

@khustochka
Copy link
Contributor

This is so cool that I will apply these to my app immediately! Many thanks

@Jacke
Copy link
Author

Jacke commented Mar 1, 2013

So funny yeah...
I think using same page for almost 9 years not cool

@Jacke Jacke closed this Mar 1, 2013
@Jacke Jacke reopened this Mar 1, 2013
@senny
Copy link
Member

senny commented Mar 1, 2013

we will need a CHANGELOG entry.

/cc @guilleiguaran @rafaelfranca

@carlosantoniodasilva
Copy link
Member

Looks nice, please do add a changelog entry, fix indenting and add a little bit of spacing so that it can get more readable. Thanks!

NARKOZ
NARKOZ reviewed Mar 1, 2013
View reviewed changes
guides/code/getting_started/public/404.html
border-top-left-radius:9px;
border-top-right-radius:9px;
background-color:white;
padding-top:7px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

padding: 7px 4em 0 4em;

@NARKOZ
Copy link
Contributor

NARKOZ commented Mar 1, 2013

422

422 page looks incomplete IMO.

@rafaelfranca
Copy link
Member

@lucasmazza could you review this one?

rafaelfranca
rafaelfranca reviewed Mar 1, 2013
View reviewed changes
guides/CHANGELOG.md
@@ -10,3 +10,4 @@
* Guides updated to reflect new test locations. *Mike Moore*

* Guides have a responsive design. *Joe Fiorini*
* Change Service pages(404, etc). *Stanislav Sobolev*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CHANGELOG entries are always on the top of file. Could you change?

@Jacke
Copy link
Author

Jacke commented Mar 1, 2013

Prefix, changelog fixed.
@NARKOZ Yeah, it incomplete, because there are have not a message for admin, like in other pages. But it can be added.

@lucasmazza
Copy link
Contributor

@Jacke the page looks nice! I left some inline comments about the code, and besides that there's a few bits that might need some work:

  • The shadows are good, but the top red bar doesn't go along too much with then. Maybe reducing the border-width or trying a different shade of red could look better.
  • As @NARKOZ pointed, the 422 page looks broken/incomplete. Maybe just that one should have a border-radius on all corners.

lucasmazza
lucasmazza reviewed Mar 1, 2013
View reviewed changes
guides/code/getting_started/public/404.html
border-bottom-right-radius:4px;
border-top-color:#DADADA;
color:#666;
box-shadow:0 3px 8px rgba(50,50,50,0.17);}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the closing bracket should be placed in the next line.

@guilleiguaran
Copy link
Member

@Jacke great work, please review all the changes mentioned and we will merge this!!

Thanks

@guilleiguaran
Copy link
Member

Please also squash your commits in a single one, thanks!!!

@Jacke Jacke closed this Mar 1, 2013
@carlosantoniodasilva carlosantoniodasilva mentioned this pull request Mar 1, 2013
Merged
@NARKOZ NARKOZ mentioned this pull request Mar 4, 2013
Merged
@milushov
Copy link

good idea for redesign server error pages 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
railties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@Jacke @khustochka @senny @carlosantoniodasilva @NARKOZ @rafaelfranca @lucasmazza @guilleiguaran @milushov