Reworked status page - Githubissues

mikepurvis commented 10 years ago

Please see demos:

Works great on Chrome and Safari, okay but a bit sluggish on Firefox—probably no hope of IE support. If it's felt to be necessary, I can put in detection to at least produce a popup warning.

Highlights are quicker loading, filtering, sorting, replaceState url, and way, way more compact page size.

A lot of small features have had to be partially or fully re-implemented, especially stuff which got moved from the Python generator to JavaScript—for regular users of this page, please give the demos a quick shot and let me know if any functionality you rely on has been lost or if something otherwise looks funky. Known lost functionality (but which could be restored upon request) includes: separate diff1/diff2/etc searching, sorting on the version block columns, and possibly more which I wasn't aware of.

There's also a ton of remaining cleanup which is possible in the Python generator. I haven't bothered to remove all of the stuff which is now no longer in use; as much as possible, I stayed focused on the front-end business of JS and CSS.

This PR addresses issue #149.

tfoote commented 10 years ago

That looks better and is much snappier.

The red1 color# queries are important. And the ability to search a specific architecture is useful.

Also the ability to get counts of matching records. (aka I search for sync and get the total number of rows at the bottom) is useful and currently lost.

dirk-thomas commented 10 years ago

The new page looks really great - and it is much faster. Than you for improving this.

I saw some minor thing which you could address please. But then I guess it will be great to deploy the changes and replace the "old" page.

the table header seems to be rendered with an offset to the right (not aligned with the column data)
re-adding the count would be helpful (N row, filtered M rows out of N) - probably somewhere at the top rather than on the bottom
place the distro name somewhere more prominent (beside being in the window title, around Debian Build Status)
at the end it would be good to remove obsolete code, that will keep it more manageable in future

mikepurvis commented 10 years ago

Thanks for the quick feedback—

Tully, is it specifically red1 which is important, or is critical to have all of them available? There are a variety of approaches which could be taken, but implementing just the red1 query can potentially be a lot simpler and lighter than a general solution. (Perhaps give it a better name, like "red-build" or "build-missing"?)

Can you also expand on what it would mean to search a specific architecture?

Yes, I'll restore the filter counts (...somewhere).

Dirk, if it's not too much trouble, could you let me know your browser and OS, and perhaps send a screenshot of what the header looks like to you?

My preference would be to refactor the Python scripts as a separate change, rather than bloating this one even further; if your preference would be to see it as an atomic switchover, though, I can do some/most of it now. The reality, though, is that a proper refurbishment of the Python would see even more of it disappear and instead become part of the empy template logic—I'm nervous to get into that work, though, as there's a lot of brittle special-case logic in functions like transform_csv_to_html, inject_status_and_maintainer, format_row, etc. Without the historical context of how and why things ended up the way they did in there, I'd be concerned about accidentally deleting some weird special case detection code that we wouldn't realise was missing until months later.

dirk-thomas commented 10 years ago

Never mind about the header offset - a clear browser session solved the issue.

Refactoring can be a separate thing. The code has grown over time and a lot of its complexity and bad readability is like due to that. I was think more about code which was used before your change and is not called anymore - that could be safely deleted.

mikepurvis commented 10 years ago

Dirk—I don't believe there's any truly dead code in the Python file. It's more a matter of functions which have parameters passed into them that aren't used any more. For example, the format_version function does way, way less than it used to, since so much of the logic is on the front end now, but it still gets passed six parameters. My preference would be to leave it as it is, and plan on a later cleanup which actually eliminates these unneeded functions entirely, rather than just stirring the pot.

You may have had cached CSS from the previous demo. If it looks like this status page may receive more regular love, we should probably add a ?xxx number to the css and js includes, so that they can be cached-busted when they change. I'll add that.

dirk-thomas commented 10 years ago

Sure, your call. I am fine with that.

mikepurvis commented 10 years ago

Okay, have updated the PR and demo:

Now shows ros distro name at top of page.
Now shows row count by the search box.
Now include a bogus querystring on the css/js includes.

Will wait on feedback from @tfoote regarding outstanding search issues.

dirk-thomas commented 10 years ago

The search results are sometimes a bit odd. E.g. I can either search for "Thomas" or "thomas" and get the same results but when I search for "Dirk" and "dirk" the later one shows zero rows. (For other first names like "william" it works though.)

mikepurvis commented 10 years ago

Yes, it's case sensitive, so sometimes you're matching email addresses with the lowercase. Case insensitivity probably makes more sense, doesn't it...

tfoote commented 10 years ago

If we could have red1, red2, red3 those are really the ones I care about. The generic is probably not necessary. If we find another common one we could probably add it too. And doing it by arch is nice especially when starting up, but hopefully we will never be in the situation where we are just starting up again.

On Tue, Oct 15, 2013 at 2:12 PM, Mike Purvis notifications@github.comwrote:

Yes, it's case sensitive, so sometimes you're matching email addresses with the lowercase. Case insensitivity probably makes more sense, doesn't it...

— Reply to this email directly or view it on GitHubhttps://github.com/ros-infrastructure/buildfarm/pull/154#issuecomment-26372712 .

mikepurvis commented 10 years ago

Okay, PR and demo updated again. Have fixed lowercase searching of names; added the red1/2/3 searches.

tfoote commented 10 years ago

It looks solid now. @dirk-thomas any other thoughts?

tfoote commented 10 years ago

oh, we do need to make sure how to deploy it correctly. I think it has a few resources it needs?

mikepurvis commented 10 years ago

I believe empy is the only new external dependency. Everything else is in the resources folder (zepto.js, and separate css/js files). The static files get copied in by the same logic which used to copy in jquery.js.

dirk-thomas commented 10 years ago

I will take care of the deployment later / tomorrow.

mikepurvis commented 10 years ago

Awesome, looks sharp. Thanks for working with me on this, and let me know if any problems arise.

mikepurvis commented 10 years ago

Fuerte page looks funky: http://www.ros.org/debbuild/fuerte.html

The issue is that the status and maintainer table cells don't seem to have been published on that one, so the first two version cells are being formatted as if they are them.

I don't think I realised debs were still being built for Fuerte. If not, that page should be disabled; if so, I can look further at the bug and try to fix it up.

dirk-thomas commented 10 years ago

Yes, Fuerte is currently still build (even if it is officially EOLed already). I have fixed the Fuerte columns with the recent commit 372efb4. Thanks for pointing it out.

dirk-thomas commented 10 years ago

I am experiencing the weird rendering issue with the table header again. But this time I can reproduce it. When I open the page (http://ros.org/debbuild/hydro.html) and then scroll down the table header shifts to the right (looks right aligned with the window).

This happens for me (and @tfoote) with Chromium (28.0.1500.71) as well as Firefox (24.0). I guess for reproducing it you need at least a 1920px wide screen. @mikepurvis Any idea what cases this and how to fix it?

dirk-thomas commented 10 years ago

When that happens the header is updated from:

<thead class="floating">

to:

<thead class="floating fixed" style="left: 142px;">

mikepurvis commented 10 years ago

That is so that the fixed header follows horizontal scrolls.

This logic here:

  // When the page scrolls, check whether the header should be fixed or floating.
  var last_left = null;
  $(window).on('scroll', function() {
    if ($(window).scrollTop() > table.position().top) {
      // Fixed thead
      header.addClass('fixed');
      var left = window.scrollX;
      left = Math.max(left, 0);
      left = Math.min(left, table.width() - document.documentElement.clientWidth);
      if (left != last_left) {
        header.css('left', -left);
        last_left = left;
      }
    } else {
      // Floating thead
      header.removeClass('fixed');
    }
  });

The min/max logic is to avoid a weird sliding thing that happens with OS X's overscroll mechanism. Try commenting those two lines out and see if it corrects the issue.

mikepurvis commented 10 years ago

Oh wait—the issue occurs when the table is narrower than the browser, of course, so table.width() - document.documentElement.clientWidth ends up negative. You could change the line to:

left = Math.min(left, Math.max(0, table.width() - document.documentElement.clientWidth));

I believe that will take care of it. I can submit a PR with this and some comments explaining what's going on there.

dirk-thomas commented 10 years ago

I will just change the line. That is definitely the reason.

dirk-thomas commented 10 years ago

Fixed - thanks again!

mikepurvis commented 10 years ago

No worries at all, thanks for the good feedback and quick merge.

ros-infrastructure / buildfarm

Reworked status page #154