Refresh PrettyPageHandler default style

[filp]

WORK IN PROGRESS - Not ready to merge just yet!

Scroll to the comments to see the latest versions of the design

This PR makes a series of changes to default stylesheet for PrettyPageHandler shipped with whoops. The goal is to refresh/modernize it a bit, have it fit larger displays better, and possibly (not yet implemented) add basic mobile support.

Here's how it looks right now:

NOTE: While it does seem like a lot of wasted space in some parts, the goal was to improve readability, and that space will collapse on smaller displays, using media queries.

@denis-sokolov @GrahamCampbell Would love your input - I haven't used whoops in a while, so it'd be good to see if you guys like the direction and think I'm hitting the right targets.

Cheers!

[prisis]

I like it 👍

[GrahamCampbell]

I think there's too much bank space before the code part starts, but otherwise, looking quite nice.

[denis-sokolov]

Welcome back, @filp!

PrettyPageHandler is the primary selling point of Whoops, thus refreshing it is a great idea!
Perhaps if you're doing changes like this, you may want to finally move the exception type and message outside of the details pane on the right? The exception details do not belong to a single stack frame, also see confusion in #252.

[filp]

This is the current state of things. I tried to address @GrahamCampbell's concerns about the spacing, and @denis-sokolov's idea to move the exception title/message out of the details panel and into its own space.

I think it looks good in the default state now, but as you scroll the details pane there's a bit of an issue with separating the different parts of the UI, specially with the dark grays overlapping - working on a solution for that.

[denis-sokolov]

We could actually move the exception name and the title to the left column, without taking space on the right. It also makes funny kind of sense - it's on the top of the stack trace.
I know it's not fully logically perfect, but it saves us a lot of space and I'm just brainstorming.

[filp]

@denis-sokolov This might work, what do you think? (Only other thing I'd do is to make it clearer what the list of frames actually is)

[filp]

And here it is with various visual tweaks - most importantly I put some attention back into the frames list to make it (I think) more clear they're inidividual items.

[denis-sokolov]

Hm. Interesting. But I'm not sure whether it's now more or less confusing.
Let me dump some thoughts, feel free to disregard them all.

The exception page has three main parts: 1. exception name and type; 2. stack trace with source code; 3. environment. So far the parts are laid out in a funny fashion:

If we streamline their layout, the stack gets too little space:

If we focus on what's important, the stacktrace and the code sample, it looks pretty good, bus it's unclear where to put the environment:

Perhaps tabs?

[filp]

Your second to last example is interesting. Again, because I haven't used whoops that much recently my idea of what regular use looks like might be skewed, but my usage of Ruby's better_errors (the inspiration for whoops) shows me I usually only need the first stack's code snippet, and occasionally need to go a few frames down - in that case, moving the env data out of the way somehow and making the code easier to read sounds good.

One option would be to not remove it completely, but push it further down and have it faded out/low-opacity (but still partially visible), and then bringing it back to full opacity as soon as you scroll down or move your mouse over it.

The issue with the different parts being laid out kinda funny is one I don't really have a solution for right now (i.e the env data technically belongs close to the exception message) - the space constraints don't really seem to work in our favor.

One counter-point to that might be the fact that environment data is interpreted next to the code (i.e something fails, you see the code that's failing and it's relation to the environment data, such as the contents of $_GET, in which case the layout might actually work quite well).

(Sorry for the slightly rambly wording, let me know if anything I said isn't clear)

[denis-sokolov]

I agree with you in that it's only the stacktrace that matters most of the time.
Ideally we'd also highlight the frames that come from the user code and collapse and deemphasize frames from library code.

I have never used the environment view. I'd be happy to put it down below the fold.
What if we press the frames together a little bit, and move the environment down the page to be scrolled up with the page, while the frames selector is in a self-scrolling container.
Perhaps something like this:

You can ensure that's a little part of the environment is always visible using vh units in CSS. The middle area can be set to, say, 85vh for big viewports, and the user will always see that he can scroll down to the environment.

[filp]

I think something like that might work. I'll try to come up with some variation of it and see how it looks.

Thanks!

[GRMule]

If you are going with tabs, you could incorporate the idea in pr #283 so the raw script output is shown as just another tab.

[denis-sokolov]

Yeah, I actually had that in one of my examples, I agree!

[staabm]

Maybe we could also increase the number of lines of context arround the offending line, when we have a lot of stackframes (because we know that the vertical space required for the stack already will be big and therefore also the space for the context lines will be there because of this fact)

[denis-sokolov]

I think we always should, actually! :)

[staabm]

Yeah, something like $linesOfContext = max(8, 2 * $numberOfStackframes)

[filp]

Hi again,

Here's the current state of the refresh. I considered the past few comments, and decided to go forward with the existing layout with a few added tweaks - while I acknowledge the fact that there's some fragmentation in how the data is grouped in the screen, I believe because of the space constraints this layout is still the one that works best.

I expanded the code section to include additional context lines (making this smarter is still an option I'm open to), and pushed the environment data further down the screen (the extra white space collapses on smaller screens).

I also gave the font weights and colors a little do-over -- there was a bit too much color in some areas and not enough contrast in others. My days as designer are long gone so I'm open to suggestions on this.

Besides that, I had previously made some small tweaks that I hadn't shared with screenshots yet, such as the bubbles on the stack frame numbers.

(Looking at the screenshots again, there's maybe a bit too much space between the code and environment data)

[denis-sokolov]

Looks great to me.

[filp]

Sweet! Any more comments or suggestions? What would be the next steps to merge this?

I'm also likely spending some time today refreshing the website at http://filp.github.io/whoops to match - I'll open a pull-request as well.

[staabm]

@filp the screen misses features like

• copy stack into clipboard
• help overlay

How will it render with longer exception messages?

[filp]

@staabm For longer exception messages, it'll cut off the message at ~200px height, and you can hover the container to expand it and see the full message. I took this idea from better_errors, and when I used it there it worked quite well.

The option to copy to clipboard is still there, but I use safari to take the screenshots and the icon doesn't show up - if I use chrome or firefox it's visible and working just fine.

The help overlay however does need to be updated to match the new layout.

[filp]

Here's the header in action ^ Pretty low-tech but serves its purpose, and the default height is enough for likely the majority of common error messages.

[GrahamCampbell]

Nice. :)

[staabm]

@filp nice, thanks for the screenshot. I like the general idea and the new layout.

One nit regarding the longer headlines: to indicate the shortened message something like a overflow: ellipsis; or a bg image visualizing a rough edge would be helpfull

[filp]

Haven't had time to return to this lately. Anyone willing to lend a hand in finishing up any missing bits so that we can go ahead and merge it?

[prisis]

Hey @filp, if you can tell me what are the missing bits, i can finish it.

[filp]

@prisis Thanks!

AFAIK the only missing bits is ensuring the "copy to clipboard" feature works properly. I've also removed the help button on the top-right. Personally I think it is unnecessary as whoops is pretty straightforward for anyone doing development - if we do keep it it'll pretty much have to be rebuilt, as I changed so much of the layout.

@staabm's comment about making the exception message cutoff more clear is also a very good point, would you like to try to come up with a solution for that?

Let me know if you have any more questions at any time!

[staabm]

regarding the clipboard stuff: I think we should re-implement it using a more modern approach in #313. If you agree we could leave this out of this PR then.

[filp]

And I believe we're done! Here's the final version:

There's certainly still room for improvement, but if you guys like it I say we move ahead and ship this - I've also updated the README to match.

@denis-sokolov How do we handle the versioning for this? The view changes are not BC.

[staabm]

really like the visual appearance. didnt check the html/css though.

will it also work with longer exception messages?

[prisis]

Can i fast fix the CSS spaces mix, Before we merge this?

[filp]

@staabm Yep, it works with longer exception messages!

@prisis Yes, please!

[filp]

@denis-sokolov How should we go about merging this? i.e re: versioning

[denis-sokolov]

@filp, I got your notification the first time! :)

Good job on getting this done. 👍
Please give me a few days to find time to strategize this, I’ve been a bit busy yesterday and today.

[denis-sokolov]

You can begin my merging/rebasing to fix the conflicts! If you don’t like conflicts much, feel free to leave it to me, I’m pretty good with that.

Merging master into your branch, I mean. Or rebasing your branch on top of master.

[filp]

@denis-sokolov Sorry, I tend to miss github notifications myself 😛

This branch is already updated with master, though I'm not sure what's missing or why github is still saying otherwise - regardless, the final merge to master should be straightforward, short of anything I'm maybe missing.

Take your time, and let me know if you need any help from my side to merge! 👍 👍 👍

[denis-sokolov]

Problem description: vast majority of users never use custom resource paths or custom css.
The changes are definitely BC breaking for the minority that do, but only in development.
We want not to bump major if possible.

We can add a hacky workaround for the minority. Store all resources before the latest changes in a /1.1 subdirectory. Then, if addResourcePath or addCustomCss is called, revert to 1.1 looks unless the user explicitly requests new version with enableLooksOf12 or something like that.

If nobody wants to do that, we'll bump the major, it's not the end of the world at all.

[hultberg]

I support increasing the major version to indicate BC @denis-sokolov 👍

[staabm]

+1 for a major version bump

[filp]

👍 major version bump. In this case it's the cleanest solution by far.

[denis-sokolov]

Hm. Overwhelming opinion. Alright.

If we bump major, I will use this opportunity to do more cleanups and deprecations in the same version.
Starting now, master represents version 2, so BC breaking PRs are welcome!
@staabm, @GrahamCampbell, @filp.

I will merge this branch in a moment.

[staabm]

Woot! 🎆

[filp]

👏 👏 👏 Thanks @denis-sokolov!

[coveralls]

Changes Unknown when pulling 69b3e06 on pretty-page-refresh into ** on master**.