[PULL REQUEST] Pull request for branch stephenfin/the-great-bootstrapification
Stephen Finucane
stephen at that.guru
Tue Oct 30 03:06:36 AEDT 2018
On Sun, 2018-10-28 at 23:57 +1100, Daniel Axtens wrote:
> Stephen Finucane <stephen at that.guru> writes:
>
> > On Thu, 2018-10-18 at 14:34 +1100, Daniel Axtens wrote:
> > > Hi Stephen,
> > >
> > > > Time to switch to Bootstrap 4. I've sent this as a pull request due to
> > > > the size of the some of the files (if various SMTP servers didn't drop
> > > > them, the mailing list would). However, there isn't a whole lot going
> > > > on here. I mostly update all the dependencies I can to the latest
> > > > version and then make the minimal amount of changes possible to handle
> > > > the Bootstrap 3 -> 4 migration.
> > > >
> > > > Please reply with comments here or against the patch on my GitHub. If
> > > > necessary, I can send this to individuals, but I'll have to do so in
> > > > Base64 encoding to work around line length problems (so you won't be
> > > > able to review this via email).
> > > >
> > >
> > > I tried to pull this but it clashes with the filter changes. Would you
> > > mind please rebasing it?
> >
> > Done and pushed to the same branch.
>
> Thanks, and thanks for keeping us up to date.
And thanks for the review. I can address almost all of the below (see
below) but I think this is the first time I've used 'git request-pull'.
What's the etiquette here w.r.t. updates? Do I force push over my
current branch and resend this mail? Push to a new branch? Push
separate fixup commits onto the current branch? Something else
entirely?
> I had been meaning to do
> reintroduce the selenium tests to test functional equivalence and update
> jQuery but I had completely forgotten that bootstrap has versions too!
Go for it. Could I ask that we namespace these things though, perhaps
by moving all the other tests into a "unit" folder and placing the
Selenium tests in a "functional" folder? I don't personally use them so
I'd rather no have to wrap tox like we used to. For reference, to run
tests I currently do:
docker-compose run --rm web tox -e py35-django20 {optional specifier}
As an aside, I've been experimenting with Vue.js lately, with the aim
of providing a second REST-powered front-end. It's been going pretty
well, some REST API bugs and feature gaps aside. Depending on how my
experiments work out, this whole process could be changing at some
point in the (distant!) future.
> I have some thoughts. This is all based on testing in Google Chrome
> stable, and I can send some annotated images if anything is confusing.
>
> Patch list page:
>
> - the table header is now dark (previously it was light). This shows
> that the margin inside the table on the left side of the P in patch
> is too small (at least on my high-dpi screen). Ideally I think I'd
> like the colours not to change, but failing that the margin needs to
> be bigger.
>
> - the table is slightly less dense, because fonts on the list page have
> changed from 14pt Helvetica Neue. The new size is 14.4pt. The new
> font is "-apple-system-font,...". I obviously don't have that on my
> Linux box - so I think I'm still on Helvetica Neue. (Anyone who has
> Roboto installed will also see a change - not sure who that is?)
I think this is because they switched to rem (root em) rather than px
or whatever they were using before. I'll experiment with this but to be
clear, I'd would really prefer to hack on Bootstrap as little as
possible. I'm no web dev and carrying lots of our own CSS sounds like
hell :D Also, Roboto would affect anyone on Android, FYI (Bootstrap is
mobile first).
> - The click to copy patch IDs (when enabled) have lost a bunch of their
> padding (top and bottom) and generally look weird and a bit broken.
>
> - all the colours are much more vivid, the colour change on hover is
> stronger, etc. I prefer the more muted pallete but I wouldn't block
> on it if it's a pain to change.
This is another one thing I'd rather not change if it's a lot of work,
even though I do agree with it in principle. Let me experiment.
> - somehow long submitter names are wrapping where previously they
> didn't. I don't have an exact reason or reproducer, but I suspect the
> font size changes are to blame.
>
> - the page and table header now sticks to the top of the screen when
> scrolling. I don't know for sure, but I reckon it's pretty likely
> that this will irritate some regular users. (It's probably also not
> *that* useful as you're probably not super likely to click those
> links often while interacting with patchwork?)
The page header was intentional, based on the design of other sites. I
can revert this though. I'm pretty sure the table header was always
sticky though?
> Patch detail page:
>
> - the patch author bar (just under "Commit Message") is too close to
> the text.
>
> - the text has been unindented and is now too close to the left side of
> the screen.
>
> - the text has shrunk and is now IMO uncomfortably small. Chrome's
> inspector tool has it going from 13pt to 12.6pt and it's amazing what
> difference 0.4pt makes!
>
> - these also affect every comment.
>
> - the patch buttons (patch ID, diff, mbox, series) have all switched to
> white on dark grey. idk how I feel about it but I want to highlight
> it in case anyone else has thoughts.
>
> - the page header sticks to the top of the screen when scrolling, and I
> think that as with the patch list page this probably isn't super
> helpful.
>
> Functionality:
>
> - the autocomplete sender lookup box seems to work, as does
> copy-to-clipboard (and filtering generally).
Hurrah! Finally :)
> - *IMPORTANT* checkboxes are kind of broken: you can no longer click
> one and then shift-click another patch above or below to select all
> patches in the range.
Again? There seems to be an issue with clickboxes.js and jQuery 3.x.
I'm probably going to end up having to re-implement this, I guess,
assuming we want jQuery 3.x :( As an aside, why can't people version
stuff properly? checkboxes.js clearly lists jQuery 3.x as supported
</rant>
> - I see a transient js error on the console when I click a link from
> the patch list page, but it doesn't seem to matter and could be
> internal to something unrelated to us?
About changing the DOM while scrolling? I saw that too. I think it's
Bootstrap itself that's raising it. Seemed like a non-issue though.
> - Show/hide headers/series seems to work.
>
> - bundle drag-and-drop reordering works (I have been a patchwork
> maintainer for I forget how long and I have only just discovered this
> page and this feature. Huh. I should check that my patch-id to msg-id
> migration doesn't break it.) The delete confirmation doesn't work but
> I don't think that's the fault of this patch set.
>
> Misc:
>
> - When creating a bundle, the top of the resultant status message gets
> hidden by the top menubar.
Good catch. I need to rework the notification system at some point.
> - There's way too much whitespace between the status message after
> creating a bundle and the content below it.
>
> Thanks again for your work keeping us up to date!
All good. Web dev is the wild west and I dislike it. Long live backend
dev.
Stephen
> Regards,
> Daniel
>
> >
> > Stephen
> >
> > > Regards,
> > > Daniel
> > >
> > > > Cheers,
> > > > Stephen
> > > >
> > > >
> > > > The following changes since commit ae154148c78a75ff73c3c22f0ff0c6b3a3d01408:
> > > >
> > > > templates: Avoid recursive call (2018-10-01 22:49:51 +0100)
> > > >
> > > > are available in the Git repository at:
> > > >
> > > > https://github.com/stephenfin/patchwork the-great-bootstrapification
> > > >
> > > > for you to fetch changes up to 6bfd03293add68db8b8de01595f94266efdd2643:
> > > >
> > > > htdocs: Remove glyphicons (2018-10-01 23:08:45 +0100)
> > > >
> > > > ----------------------------------------------------------------
> > > > Stephen Finucane (10):
> > > > htdocs: Move all jQuery files from 'lib'
> > > > htdocs: Fix formatting issues with README
> > > > htdocs: Add and integrate Font Awesome
> > > > htdocs: Update checkboxes.js to v1.2.2
> > > > htdocs: Update StickyTableHeaders to 0.1.24
> > > > htdocs: Update jQuery to v3.3.1
> > > > htdocs: Update selectize.js to v0.12.4
> > > > templates: Upgrade to Bootstrap 4
> > > > htdocs: Update Bootstrap to v4.1.3
> > > > htdocs: Remove glyphicons
> > > >
> > > > htdocs/README.rst | 57 +-
> > > > htdocs/css/bootstrap.min.css | 8 +-
> > > > htdocs/css/bootstrap.min.css.map | 1 +
> > > > htdocs/css/fontawesome.min.css | 5 +
> > > > htdocs/css/selectize.bootstrap3.css | 401 ----
> > > > htdocs/css/selectize.bootstrap4.css | 376 +++
> > > > htdocs/css/solid.min.css | 5 +
> > > > htdocs/css/style.css | 63 +-
> > > > htdocs/fonts/glyphicons-halflings-regular.eot | Bin 20335 -> 0 bytes
> > > > htdocs/fonts/glyphicons-halflings-regular.svg | 229 --
> > > > htdocs/fonts/glyphicons-halflings-regular.ttf | Bin 41280 -> 0 bytes
> > > > htdocs/fonts/glyphicons-halflings-regular.woff | Bin 23320 -> 0 bytes
> > > > htdocs/js/bootstrap.min.js | 11 +-
> > > > htdocs/js/bootstrap.min.js.map | 1 +
> > > > htdocs/js/jquery-1.10.1.min.js | 1 -
> > > > htdocs/js/jquery-3.3.1.min.js | 2 +
> > > > htdocs/js/jquery.checkboxes-1.0.6.min.js | 1 -
> > > > htdocs/js/jquery.checkboxes-1.2.2.min.js | 1 +
> > > > htdocs/js/jquery.stickytableheaders.min.js | 7 +-
> > > > htdocs/js/jquery.tablednd.js | 315 ++-
> > > > htdocs/js/popper.min.js | 5 +
> > > > htdocs/js/selectize.min.js | 7 +-
> > > > htdocs/webfonts/fa-solid-900.eot | Bin 0 -> 180720 bytes
> > > > htdocs/webfonts/fa-solid-900.svg | 2444 ++++++++++++++++++++
> > > > htdocs/webfonts/fa-solid-900.ttf | Bin 0 -> 180500 bytes
> > > > htdocs/webfonts/fa-solid-900.woff | Bin 0 -> 86876 bytes
> > > > htdocs/webfonts/fa-solid-900.woff2 | Bin 0 -> 67400 bytes
> > > > lib/packages/.gitignore | 1 -
> > > > lib/packages/jquery/README | 16 -
> > > > lib/packages/jquery/jquery-1.10.1.min.js | 6 -
> > > > lib/packages/jquery/jquery.checkboxes-1.0.6.min.js | 1 -
> > > > .../jquery/jquery.stickytableheaders.min.js | 1 -
> > > > lib/packages/jquery/jquery.tablednd.js | 314 ---
> > > > patchwork/filters.py | 33 +-
> > > > patchwork/templates/patchwork/about.html | 32 +-
> > > > patchwork/templates/patchwork/bundles.html | 11 +-
> > > > .../patchwork/partials/download-buttons.html | 14 +-
> > > > .../templates/patchwork/partials/filters.html | 37 +-
> > > > .../templates/patchwork/partials/patch-list.html | 36 +-
> > > > patchwork/templates/patchwork/projects.html | 42 +-
> > > > patchwork/templates/patchwork/submission.html | 4 +-
> > > > templates/base.html | 169 +-
> > > > 42 files changed, 3415 insertions(+), 1242 deletions(-)
> > > > create mode 100644 htdocs/css/bootstrap.min.css.map
> > > > create mode 100644 htdocs/css/fontawesome.min.css
> > > > delete mode 100644 htdocs/css/selectize.bootstrap3.css
> > > > create mode 100644 htdocs/css/selectize.bootstrap4.css
> > > > create mode 100644 htdocs/css/solid.min.css
> > > > delete mode 100644 htdocs/fonts/glyphicons-halflings-regular.eot
> > > > delete mode 100644 htdocs/fonts/glyphicons-halflings-regular.svg
> > > > delete mode 100644 htdocs/fonts/glyphicons-halflings-regular.ttf
> > > > delete mode 100644 htdocs/fonts/glyphicons-halflings-regular.woff
> > > > create mode 100644 htdocs/js/bootstrap.min.js.map
> > > > delete mode 120000 htdocs/js/jquery-1.10.1.min.js
> > > > create mode 100644 htdocs/js/jquery-3.3.1.min.js
> > > > delete mode 120000 htdocs/js/jquery.checkboxes-1.0.6.min.js
> > > > create mode 100644 htdocs/js/jquery.checkboxes-1.2.2.min.js
> > > > mode change 120000 => 100644 htdocs/js/jquery.stickytableheaders.min.js
> > > > mode change 120000 => 100644 htdocs/js/jquery.tablednd.js
> > > > create mode 100644 htdocs/js/popper.min.js
> > > > create mode 100644 htdocs/webfonts/fa-solid-900.eot
> > > > create mode 100644 htdocs/webfonts/fa-solid-900.svg
> > > > create mode 100644 htdocs/webfonts/fa-solid-900.ttf
> > > > create mode 100644 htdocs/webfonts/fa-solid-900.woff
> > > > create mode 100644 htdocs/webfonts/fa-solid-900.woff2
> > > > delete mode 100644 lib/packages/.gitignore
> > > > delete mode 100644 lib/packages/jquery/README
> > > > delete mode 100644 lib/packages/jquery/jquery-1.10.1.min.js
> > > > delete mode 100644 lib/packages/jquery/jquery.checkboxes-1.0.6.min.js
> > > > delete mode 100644 lib/packages/jquery/jquery.stickytableheaders.min.js
> > > > delete mode 100644 lib/packages/jquery/jquery.tablednd.js
> > > >
> > > > _______________________________________________
> > > > Patchwork mailing list
> > > > Patchwork at lists.ozlabs.org
> > > > https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list