WebKit / Speedometer

An open source repository for the Speedometer benchmark
Other
592 stars 70 forks source link

Add separate about.html and instructions.html #359

Closed camillobruni closed 7 months ago

camillobruni commented 7 months ago

Adds a basic instructions section. To be extended once the document in #352 is filled in.

Screenshot 2024-01-25 at 11 43 32 Screenshot 2024-01-25 at 11 43 54
julienw commented 7 months ago

Is it ready for review or are you waiting for the document? If it's not ready for review, it would be good to be explicit and mark the PR as a draft :-) Thanks

camillobruni commented 7 months ago

Let's try to land this already so everything is in place and we only need to update the content.

camillobruni commented 7 months ago

I forgot to redirect the #running view to #home by default if there are no results yet.

bgrins commented 7 months ago

I forgot to redirect the #running view to #home by default if there are no results yet.

I know it was added for a reason, but I wonder if we should reconsider the hash navigation.

It seems to me the benefit would be if we wanted to allow users to drive UI states with the back button - and in general we don't, in the sense that you can't go back from #summary to #running by design. If we don't need that property there would be simpler ways to manage the UI state that avoid these types of bugs: just set some kind of attribute on the body to toggle display for the corresponding section (or even toggling an attribute on the sections themselves).

One thing that's nice about the current behavior is that you can deep link to the About dialog, but we could handle that as a special case (reading location.hash at page load and toggling the about dialog if it matches "#about"). Any thoughts on this @camillobruni @rniwa ? Not volunteering Camillo to make that change, and I don't think it's a blocker, but if we generally agree on this then maybe @julienw or I could work on a patch if you'd like.

julienw commented 7 months ago

In addition to the good suggestion from bgrins, I was also thinking that all the instructions and about texts could be in a separate plain HTML page, so that the runner would stay fairly simple with its 4 states (home / running / details / summary).

rniwa commented 7 months ago

In addition to the good suggestion from bgrins, I was also thinking that all the instructions and about texts could be in a separate plain HTML page, so that the runner would stay fairly simple with its 4 states (home / running / details / summary).

Yeah, that could be a simpler solution overall.

bgrins commented 7 months ago

I agree a separate page may be a bit simpler, but the test instructions are quite contextual and make at least as much sense in the main UI as About does (IMO). But if we do want to move it to a new page, I'd suggest we make a markdown file in GitHub and link to it so that (a) we don't have to figure out styling for the new page and (b) we can update the text "off-train" and not require a version bump if we want to include a new instruction.

camillobruni commented 7 months ago

I'm fine either way. Maybe hosting the instructions and about text as separate documents is solving most complexity of the hash-based navigation. I'll create WIP PR with that change.

camillobruni commented 7 months ago

I've now moved the about and instructions sections into standalone .html files which makes the navigation logic much simpler.