migrate to Bootstrap v4 #773
Conversation
|
@jarun once the UI changes are finalized, I can add these screenshots to the docs repo and replace images in bukuserver readme Feel free to suggest changes in the meantime |
| /* overriding icon-button text color with theme color */ | ||
| form.icon button { | ||
| color: inherit; | ||
| } |
There was a problem hiding this comment.
This fixes the Delete button in bookmarks/tags lists not matching theme color
| @@ -5,7 +5,7 @@ body.modal-open { | |||
|
|
|||
| /* limit dialog height with a scrollbox */ | |||
| .modal-content { | |||
| max-height: calc(100vh - 60px); | |||
| max-height: calc(100vh - 3.5rem); | |||
There was a problem hiding this comment.
Bootstrap4 uses rem (root element font size) as the primary size unit
| alert(`Invalid page size: "${page}"`); | ||
| return false; | ||
| }); | ||
| $('a', this).last().clone().text("custom").attr('href', `#`).on('click', () => { |
There was a problem hiding this comment.
Dropdown items aren't nested within <li>s in Bootstrap4 templates
| @@ -6,6 +6,7 @@ | |||
| {{ buku.limit_navigation_if_popup() }} | |||
| {{ buku.script('bookmark.js') }} | |||
| {{ buku.fetch_checkbox() }} | |||
| {{ buku.horizontal_form(excluding_popups=True) }} | |||
There was a problem hiding this comment.
As I mentioned, all forms were originally vertical in Bootstrap4 templates
| {{ buku.focus() }} | ||
| {{ buku.link_saved() }} | ||
| <script>$('.submit-row').append($('.delete-form'))</script> |
There was a problem hiding this comment.
Adding the Delete button was moved to before making the form horizontal (which in turn is placed before changing keyboard focus)
| <form class="navbar-form navbar-right" action="{{ url_for('bookmark.index_view') }}" method="GET"> | ||
| <div class="form-group"> | ||
| <form class="form-inline navbar-right" style="gap:.5ex" action="{{ url_for('bookmark.index_view') }}" method="GET"> | ||
| <div class="d-inline-block align-middle"> |
There was a problem hiding this comment.
navbar-form was removed in Bootstrap4; and form-group doesn't have the desired effect here.
| <input type="text" class="form-control" id="inputKeywords" placeholder="Search bookmark" name="flt1_buku_search"> | ||
| </div> | ||
| <button type="submit" class="btn btn-default">Search</button> | ||
| <button type="submit" class="btn btn-secondary">Search</button> |
There was a problem hiding this comment.
-default got renamed to -secondary in Bootstrap4
| </form> | ||
| {% endblock %} | ||
|
|
||
| {% block body %} | ||
| {{ super() }} | ||
| <main class="container text-center"> | ||
| <div style="padding: 40px 15px"> | ||
| <main class="container text-center p-4"> |
There was a problem hiding this comment.
Using native Bootstrap4 padding classes
| <h1>BUKU</h1> | ||
| <p class="lead">Bookmark manager like a text-based mini-web</p> | ||
| <p> | ||
| <a class="btn btn-lg btn-success" href="{{ url_for('bookmark.index_view') }}" role="button">Bookmarks</a> | ||
| <a class="btn btn-lg btn-success" href="{{ url_for('tag.index_view') }}" role="button">Tags</a> | ||
| <a class="btn btn-lg btn-success" href="{{ url_for('statistic.index') }}" role="button">Statistics</a> | ||
| </p> | ||
| <div class="col-md-4 col-md-offset-4"> | ||
| <div class="col-md-4 offset-md-4"> |
There was a problem hiding this comment.
Spacing classes got renamed as well
| <div class="checkbox"> {{form.regex()}} {{form.regex.label}} </div> | ||
| <div class="text-left"> | ||
| <div class="form-check"> {{form.deep()}} {{form.deep.label}} </div> | ||
| <div class="form-check"> {{form.regex()}} {{form.regex.label}} </div> |
There was a problem hiding this comment.
checkbox got renamed, and the new class adds spacing as well
| .popup .nav-tabs :is(:first-child, :nth-child(2):not(.active)) > * {display: none} | ||
| .popup #admin-navbar-collapse {display: block} | ||
| .popup :is(.navbar-nav, .navbar-toggler) {display: none} | ||
| .popup .nav-tabs :is(:first-child, :nth-child(2)) > .nav-link:not(.active) {display: none} |
There was a problem hiding this comment.
#admin-navbar-collapse is now the style of the entire header (it simply changes display: with page width); and the overall design of navbar/tabs got changed as well
| @@ -31,17 +32,37 @@ | |||
| {% endif %} | |||
| {% endmacro %} | |||
|
|
|||
| {% macro fetch_checkbox(checked=True) %} | |||
| {% macro fetch_checkbox(checked=True, modal=False) %} | |||
There was a problem hiding this comment.
modal= is used for placing the Fetch checkbox within the buttons row in modals (as per the screenshots)
| $('.admin-form fieldset{% if modal %} .modal-footer{% endif %}').{% if modal %}prepend{% else %}append{% endif %}( | ||
| $(`<div class="form-group {{ 'mb-0' if modal else '' }}"{% if modal %} style="flex-grow: 1"{% endif %}>` | ||
| +`<label class="control-label {{ 'mb-0' if modal else '' }}">{{ _gettext('Fetch') }} </label>` | ||
| +`<input type="checkbox" name="fetch"{% if checked %} checked{% endif %}></div>`)); |
There was a problem hiding this comment.
Rearranging it similarly to how checkbox inputs are generated by flask-admin in Bootstrap4 templates
| <script> | ||
| $('.admin-form .form-group').each(function () { | ||
| if ($('.submit-row', this).length > 0{% if excluding_popups %} || document.body.matches('.popup'){% endif %}) { | ||
| $('.submit-row', this).addClass(document.body.matches('.popup') ? 'col-md-12' : 'offset-md-2'); |
There was a problem hiding this comment.
Within popups, the buttons row is made full-width instead (flask-admin pads the row instead, but it uses wrong CSS class name 😅)
| } else if ($('input[type=checkbox], input[type=radio]', this).length > 0) { | ||
| $('label', this).addClass('form-row').css({cursor: 'pointer'}).html(`<span>${ $('label', this).html() }</span>`); | ||
| $('label span', this).addClass('col-md-2 col-form-label text-right d-inline-block'); | ||
| $('input', this).appendTo($('label', this)); |
There was a problem hiding this comment.
Checkboxes/radiobuttons need custom rearrangement
| </script> | ||
| {% endmacro %} | ||
|
|
||
| {% macro details_formatting(prefix='') %} | ||
| <script> | ||
| $(`.modal-header h3`).wrapInner('<h5 class="modal-title">').children(0).unwrap(); // flask-admin #2505 |
There was a problem hiding this comment.
This can be removed once the related bug is fixed in upstream (…and ends up in the released version, of course)
| </form> | ||
| <h3>Netloc</h3> | ||
|
|
||
| {% if netlocs %} | ||
| <div> | ||
| <div class="row"> |
There was a problem hiding this comment.
.row is now required for .col- classes to work
| <div class="col-md-6"> | ||
| <canvas id="mostCommonChart" width="500" height="500"></canvas> | ||
| </div> | ||
|
|
||
| <div class="col-md-6"> | ||
| {% if netlocs.cropped %} | ||
| <button type="button" class="btn btn-primary btn-xs" data-toggle="modal" data-target="#netlocModal"> | ||
| <button type="button" class="btn btn-primary btn-sm" data-toggle="modal" data-target="#netlocModal"> |
There was a problem hiding this comment.
.btn-xs got removed, with suggestion to use .btn-sm instead
| <h4 class="modal-title" id="myModalLabel">Netloc ranking</h4> | ||
| <button type="button" class="close" data-dismiss="modal" aria-label="Close"><span aria-hidden="true">×</span></button> |
There was a problem hiding this comment.
Bootstrap4 uses flexbox instead of float: for dialog close button positioning
| <table class="table table-condensed"> | ||
| <thead class="bg-primary"> | ||
| <table class="table table-sm"> | ||
| <thead class="thead-dark"> |
There was a problem hiding this comment.
.table-condensed got renamed for sake of consistency; .thead-dark is the table header class that matches the navbar colors
| @@ -248,7 +248,7 @@ <h4 class="modal-title" id="myModalLabel">Common titles ranking</h4> | |||
| {{ super() }} | |||
| <script>// sticky table headers in modals | |||
| $('.modal-body').each(function () { | |||
| $('thead', this).css({top: '-' + $.css(this, 'padding-top')}); | |||
| $('thead', this).css({top: `calc(-${$.css(this, 'padding-top')} - 1px)`}); // +border | |||
There was a problem hiding this comment.
Table headers now have a border (which is not affected by position: sticky)
| <form id="refresh" class="d-none" method="POST" action="refresh"></form> | ||
| <li class="nav-item"> | ||
| <a class="nav-link" href="#" onclick="refresh.submit()">{{ _gettext('Refresh') }}</a> | ||
| </li> |
There was a problem hiding this comment.
.hidden got renamed to match other display: classes; also, tab buttons now require .nav-item/``.nav-link`
| @@ -628,7 +629,7 @@ def format_value(field, bookmark, spacing=''): | |||
|
|
|||
| def link(text, url, new_tab=False, html=False, badge=''): | |||
| target = ('' if not new_tab else ' target="_blank"') | |||
| cls = ('' if not badge else f' class="btn label label-{badge}"') | |||
| cls = ('' if not badge else f' class="btn badge badge-{badge}"') | |||
There was a problem hiding this comment.
.label got renamed to .badge in Bootstrap4
| @@ -149,7 +149,7 @@ def test_bookmarklet_view(bukudb, client, exists, uri, tab, args): | |||
|
|
|||
| response = client.get('/bookmarklet', query_string=query, follow_redirects=True) | |||
| dom = assert_response(response, uri, argnames=args) | |||
| assert dom.xpath(f'//ul{xpath_cls("nav nav-tabs")}/li{xpath_cls("active")}/a/text()') == [tab] | |||
| assert dom.xpath(f'//ul{xpath_cls("nav nav-tabs")}//a{xpath_cls("nav-link active")}/text()') == [tab] | |||
There was a problem hiding this comment.
Reflecting the changes in tabs HTML
LeXofLeviafan
force-pushed
the
bootstrap4
branch
from
2ddcb64 to
de1bd37
Compare
LeXofLeviafan
force-pushed
the
bootstrap4
branch
2 times, most recently
from
34ca66d to
056b0df
Compare
LeXofLeviafan
force-pushed
the
bootstrap4
branch
from
056b0df to
18313f8
Compare
LeXofLeviafan
force-pushed
the
bootstrap4
branch
from
18313f8 to
dd05421
Compare
| </main> | ||
| {% endblock %} | ||
|
|
||
| {% block tail %} | ||
| {{ buku.set_lang() }} | ||
| <script> | ||
| $(`[data-toggle="tooltip"]`).attr('data-html', 'true').each(function () { | ||
| this.title = this.title.replace(/'(.*?)'/g, `'<strong><tt>$1</tt></strong>'`) | ||
| this.title = this.title.replace(/'(.*?)'/g, `'<strong><code>$1</code></strong>'`) |
There was a problem hiding this comment.
<tt> gets removed by sanitizer (along with its contents)
Finally finished going through all the hairy bits for Bootstrap migration (or at least it appears decent enough for merging).
Here's my screenshot samples, each in 4 versions (for sake of comparison):
defaultthemedefaultthemeslatethemeslatethememain page
(note that both base font size and some colors/components have changed in v4 themes)
main page (expanded)
bookmarks list
(the iconset got changed between Bootstrap versions)
bookmarks list (filtered)
(pagination panel is cut off because the page contents is now taller than my browser window due to changed sizes)
bookmarklet popup (create bookmark)
(I increased the popup height to account for changed sizes)
(before increasing popup height, the Fetch checkbox was pushed out of the view)
bookmarklet popup (edit bookmark)
(even with the recently increased width, the Delete button barely fits within the row in popups… and in some themes it doesn't)
bookmarklet popup (view bookmark)
bookmark create dialog
(the buttons row change is not my doing; I did, however, figure out I might as well put the Fetch checkbox inside it)
(also note that all forms are vertical in Bootstrap4 templates of
flask-admin; I modified them to horizontal to reduce design changes, but it's easy to turn off in the code)(the Tags input looks different because it's added on top of Bootstrap proper, and is thus mostly unaffected by themes)
bookmark edit dialog
bookmark view dialog (random entry)
(the filter design change is not my doing either)
bookmark create page (accessible from bookmark view/edit page)
(the sizing changes appear to be the cause of the input fields sticking out slightly from the right side… though as I mentioned all forms were originally vertical in Bootstrap4 templates)
bookmark edit page (accessible from bookmark view page)
bookmark view page (accessible via link in view dialog / "bookmark saved" message)
tags list
(note that the "page 2" button is hovered in these screenshots)
tag edit page
success message ("bookmark saved", linked to the saved record)
error message ("invalid filter value")
stats page
(the table is now taller due to changed sizes)
stats page (chart hovered)
stats page (full list dialog)
(…the dialog table appears thinner now as well)
(also I changed header color here to match the navbar)
(I've checked them out in other themes as well – they appear serviceable at least)
Reason for keeping it a draft pull-request (for the time being): a minor bug caused by upstream, fixed in their repo but not in their current PyPI version (the brunt of it was fixed by #760, but the spammed requests still have a side-effect of dialogs freezing when opened for increasingly longer durations).
Because of this bug, I suggest delaying the merge until the new version of
flask-adminis released (so that we can finish version migration soon after that). I also suggest creating a (non-release) tag immediately before merging (e.g.archive/pre-bootstrap4), for sake of having it easier to locate if needed.