[jarun#753] pre-migration WebUI fixes #761

LeXofLeviafan · 2024-08-23T23:45:41Z

As I mentioned, I've been working on Bootstrap v4 migration; while doing so, I made a number of fixes and adjustments that aren't strictly related to Bootstrap v4, so I made a separate pull-request for them to be merged before migrating to the newer Bootstrap version.

This includes the following changes:

preventing the spamming of /api/tags query if possible (see also [jarun#753] implement thread safety #760)
refactoring the Stats page (in particular, making "View all" stats dialogs consistent with charts – it makes no sense for them to have less entries – and fixing dialogs with long tables)
some minor UX improvements, fixes and code refactoring
- removing Flask-Bootstrap dependency (which is never actually used… and also only supports Bootstrap up to v3)
- making the bookmarklet popup window wider (it can't fit all buttons when editing an existing record)
- switching from custom CSS to Bootstrap classes where possible
- using Bootstrap badges ("labels" in v3) to display tag links + highlighting the netloc badge with a different colour
- changing entry cells in the Bookmarks page to rely on CSS for formatting
- implementing live recalculation of the "Data created ... seconds ago" caption in Stats page (also switching to larger time units when appropriate)
- improving the fake "no tags" entry in the Tags page
- refactoring views.py and functional tests in test_views.py

LeXofLeviafan · 2024-08-23T23:51:34Z

bukuserver/bookmarklet.js

@@ -16,4 +16,4 @@ url = "{{url}}" +
    "?url=" + encodeURIComponent(url) +
    "&title=" + encodeURIComponent(title) +
    "&description=" + encodeURIComponent(desc);
-window.open(url, '_blank', 'menubar=no, height=600, width=600, toolbar=no, scrollbars=yes, status=no, dialog=1');
+window.open(url, '_blank', 'menubar=no, height=600, width=800, toolbar=no, scrollbars=yes, status=no, dialog=1');


600px width is not enough when opening an existing bookmark:

LeXofLeviafan · 2024-08-23T23:52:14Z

bukuserver/server.py

@@ -120,7 +119,6 @@ def shell_context():

    app.jinja_env.filters['netloc'] = lambda x: urlparse(x).netloc  # pylint: disable=no-member

-    Bootstrap(app)


We're using Bootstrap bundled with flask-admin instead

LeXofLeviafan · 2024-08-23T23:56:13Z

bukuserver/static/bukuserver/css/bookmark.css

+
+.description {
+    white-space: pre-wrap;
+}


All these are blocks now; .tag-list also now has gaps between its elements (font-size: is for badges), and .description retains whitespace as-is (other than wrapping long lines)

(note: the last "netloc" is actually a tag)

LeXofLeviafan · 2024-08-24T00:05:39Z

bukuserver/static/bukuserver/css/statistic.css

+/* overriding icon-button text color with theme color */
+.modal-header :is(button, button:hover) {
+    color: inherit;
+}


Preventing Stats dialog screen overflow (replacing it with a scrollbox), making the table header sticky, and ensuring the close button is visible regardless of UI theme

LeXofLeviafan · 2024-08-24T00:06:23Z

bukuserver/static/bukuserver/js/bookmark.js

+                    window._tagsQueried = Date.now();
+                    $.getJSON('/api/tags', ({tags}) => resolve(tags));
+                  }));
+  _tags.then(tags => $('input#tags').select2({tags, tokenSeparators: [',']}));


This avoids sending more than one /api/tags request per second

LeXofLeviafan · 2024-08-24T00:08:22Z

bukuserver/templates/bukuserver/home.html

-          <div class="form-group"> {{form.regex()}} {{form.regex.label}} </div>
+        <div class="text-left col-sm-offset-3">
+          <div class="checkbox"> {{form.deep()}} {{form.deep.label}} </div>
+          <div class="checkbox"> {{form.regex()}} {{form.regex.label}} </div>


Applying the Bootstrap theme, with minimum layout changes

LeXofLeviafan · 2024-08-24T00:09:04Z

bukuserver/templates/bukuserver/lib.html

-      $(`<label class="form-group" style="display: block"><span class="col-md-2 text-right">{{ _gettext('Fetch') }}</span>`
-        +`<span class="col-md-10"><input type="checkbox" name="fetch"{% if checked %} checked{% endif %}></span></label>`))
+      $(`<div class="form-group"><label style="display: block"><span class="col-md-2 text-right">{{ _gettext('Fetch') }} &nbsp; </span>`
+        +`<span class="col-md-10"><input type="checkbox" name="fetch"{% if checked %} checked{% endif %}></span></label></div>`));


This is closer to how other inputs in the form are arranged

LeXofLeviafan · 2024-08-24T00:16:39Z

bukuserver/templates/bukuserver/statistic.html

+      created.innerText = (!unit ? "just now" : `${parseInt(diff/UNITS[unit])} ${unit}s ago`);
+      setTimeout(recalcReltime, 1000 * {second: 1, minute: 15, hour: 60, day: 15*60}[unit||'second']);
+    });
+  </script>


Live recalculation of displayed relative time (with appropriate units)

I'd say this is easier to wrap the mind around than "very many seconds" you'd see otherwise 😅

(Refresh rate is: every second when showing time in seconds, four times a minute when showing minutes, once a minute when showing hours, and every quarter of hour when showing days. The reason to have steps shorter than the unit is to reduce discrepancy between displayed and actual time, since the timer is not reset when reopening/refreshing the page.)

LeXofLeviafan · 2024-08-24T00:17:33Z

bukuserver/templates/bukuserver/statistic.html

-                  {% else %}
-                  <span class="btn btn-default" disabled="disabled">(No Netloc)</span>
-                  {% endif %}
+                  <a href="{{ buku.filter('url_netloc_match', name) }}">{{ name or '(no netloc)' }}</a>


There's no reason to disallow searching for entries with no netloc 😅

LeXofLeviafan · 2024-08-24T00:18:06Z

bukuserver/templates/bukuserver/statistic.html

-                  {% else %}
-                  <span class="btn btn-default" disabled="disabled">(No Title)</span>
-                  {% endif %}
+                  <a href="{{ buku.filter('title_equals', name) }}">{{ name or '(no title)' }}</a>


There's no reason to disallow searching for entries with no title 😅

LeXofLeviafan · 2024-08-24T00:19:10Z

bukuserver/templates/bukuserver/statistic.html

+    $('.modal-body').each(function () {
+      $('thead', this).css({top: '-' + $.css(this, 'padding-top')});
+    });
+  </script>


Sticky header top: must equal the (negative) distance to the top of the scrollbox

LeXofLeviafan · 2024-08-24T00:20:01Z

bukuserver/templates/bukuserver/statistic.html

  {{ buku.script('Chart.js') }}
+  {% set NO_NETLOC = '\u200B(no netloc)\u200B' %}


Handling the blank name in the chart (it should be displayed, same as in table, but searching should be done against an empty string)

LeXofLeviafan · 2024-08-24T00:20:14Z

bukuserver/templates/bukuserver/statistic.html

-  var netlocCtx = document.getElementById("mostCommonChart").getContext('2d');
-  var netlocChart = new Chart(netlocCtx, {
+  var netlocCtx = document.getElementById("mostCommonChart")?.getContext('2d');
+  var netlocChart = netlocCtx && new Chart(netlocCtx, {


Avoiding errors when the chart is not rendered

LeXofLeviafan · 2024-08-24T00:20:33Z

bukuserver/templates/bukuserver/statistic.html

@@ -333,8 +363,6 @@ <h4 class="modal-title" id="myModalLabel">Title ranking</h4>
      }
    }
  });
-
-  titleChart.canvas.parentNode.style.height = '128px';


This wasn't doing anything other than causing rendering issues in some cases 😅

LeXofLeviafan · 2024-08-24T00:23:04Z

bukuserver/views.py

@@ -430,8 +418,8 @@ def _apply_filters(self, models, filters):
    def _name_formatter(self, _, model, name):
        data = getattr(model, name)
        query, title = (({'flt0_tags_contain': data}, data) if data else
-                        ({'flt0_tags_number_equal': 0}, '<EMPTY TAG>'))
-        return Markup(f'<a href="{escape(url_for("bookmark.index_view", **query))}">{escape(title)}</a>')
+                        ({'flt0_tags_number_equal': 0}, '<UNTAGGED>'))


Renaming for clarity

LeXofLeviafan · 2024-08-24T00:24:25Z

bukuserver/templates/bukuserver/tags_list.html

+{% block tail %}
+  {{ super() }}
+  <script>$('tr:has(a[href$="/bookmark/?flt0_tags_number_equal=0"]) .list-buttons-column').html('')</script>
+{% endblock %}


Removing action buttons for the "no tags" tag

LeXofLeviafan · 2024-08-24T00:27:08Z

bukuserver/views.py

+            tags=CountedData(data['tags']),
+            titles=CountedData(data['titles']),
+            datetime=datetime,
+            datetime_text=datetime.humanize(arrow.now(), granularity='second'),
        )


Refactoring stats code to simplify it and remove logic duplication.

Also moved amount filtering here from the UI, and modified it slightly (there's no need to exclude netlocs with 1 URL, but the only good reason to have a "ranking" for bookmark titles is looking for duplicates)

LeXofLeviafan · 2024-08-24T00:29:08Z

bukuserver/views.py

+
+def sorted_counter(keys, *, min_count=0):
+    data = Counter(keys)
+    return Counter({k: v for k, v in sorted(data.items()) if v > min_count})


Same-amount entries in the Stats tables will be ordered lexicographically now

LeXofLeviafan · 2024-08-24T00:29:54Z

bukuserver/templates/bukuserver/statistic.html

          </table>
        </div>
      </div>
    </div>
  </div>
  {% endif %}

-  <h3 class="col-md-12">Title</h3>
+  <h3 class="col-md-12">Title (common)</h3>


Reflecting exclusion of unique titles in displayed captions

LeXofLeviafan · 2024-08-24T00:30:55Z

bukuserver/views.py

-        super().__init__(*args, **kwargs)
+        self.bukudb = bukudb
+        custom_model = types.SimpleNamespace(bukudb=bukudb, __name__='bookmark')
+        super().__init__(custom_model, *args, **kwargs)


A simpler way of doing this

LeXofLeviafan · 2024-08-24T00:31:44Z

bukuserver/views.py

@@ -715,4 +624,30 @@ def filter_key(flt, idx=''):

 def format_value(field, bookmark, spacing=''):
    s = bookmark[field.value]
-    return s if field != BookmarkField.TAGS else s.strip(',').replace(',', ','+spacing)
+    return s if field != BookmarkField.TAGS else (s or '').strip(',').replace(',', ','+spacing)


This was causing occasional errors during form parsing

LeXofLeviafan · 2024-08-24T00:33:47Z

bukuserver/views.py

+def link(text, url, new_tab=False, badge=''):
+    target = ('' if not new_tab else ' target="_blank"')
+    cls = ('' if not badge else f' class="btn label label-{badge}"')
+    return f'<a{cls} href="{escape(url)}"{target}>{escape(text)}</a>'


Rendering hyperlinks with an utility for simpler and more consistent handling

LeXofLeviafan · 2024-08-24T00:36:27Z

tests/test_views.py

-        assert cell == (prefix + f'<br/><a href="{url}"{target}>{url}</a><br/>' +
-                        f'<a class="btn btn-default" href="/bookmark/?flt0_url_netloc_match={netloc}">netloc:{netloc}</a>' +
-                        suffix)
+        assert cell == f'{prefix}<span class="link"><a href="{url}"{target}>{url}</a></span>{suffix}'


Note that there's no <br/>s now; tags and description are in <div>s instead, and .link has display: block.

Also, every component has its own DOM node (tagged with an appropriate CSS class), allowing for further CSS-only formatting.

LeXofLeviafan · 2024-08-24T00:38:18Z

tests/test_views.py

@@ -109,6 +109,7 @@ def test_bmv_create_form(bmv_instance, url, backlink, app):
 #

 xpath_alert = lambda kind, message: f'//div[@class="alert alert-{kind} alert-dismissable"][contains(., "{message}")]'
+xpath_cls = lambda s: ''.join(f'[contains(concat(" ", @class, " "), " {s} ")]' for s in s.split(' ') if s)


@class= literally just checks for string equality; this matches separate class-names instead (like a selector query would)

LeXofLeviafan · 2024-08-24T00:39:57Z

tests/test_views.py

-    assert dom.xpath('//head/link[@rel="stylesheet"][@href=$href]',
-                     href=f'/static/admin/bootstrap/bootstrap3/swatch/{theme or "default"}/bootstrap.min.css?v=3.3.5')
+    assert dom.xpath('//head/link[@rel="stylesheet"][starts-with(@href, $href)]',
+                     href=f'/static/admin/bootstrap/bootstrap3/swatch/{theme or "default"}/bootstrap.min.css?')


We don't actually care about precise version numbers here; we're only checking the theme name

jarun · 2024-08-24T02:35:19Z

Thank you!

Please let me know if you have any more immediate fixes/improvements in mind. Otherwise, I'll plan for a release.

LeXofLeviafan · 2024-08-24T03:05:49Z

I actually have the Bootstrap v4 changes more or less ready (sans screenshots), but if we merge those in we'll need to wait for the flask-admin release (since there's at least one bug that was fixed in upstream after the current release).

I'm actually not sure why they're stalling the release (they've been quiet for 2 weeks now), but I'm pretty sure it's going to happen sooner rather than later… and we'll probably need to make a release after that regardless of whether we release Bootstrap v3 changes now. (If you don't see a problem with that, then making a release before migration seems like a good idea.)

Other than that, I only had a thought about adjusting the title/name filter (in Bookmarks/Tags pages, respectively) so that it actually allows for partial match – the filter is nearly useless without it (for manual usecase).

P.S. Have you seen my question about migrating dev team posts? I don't seem to have the access to do it (which makes sense but then why do I have the button? 😅)

jarun · 2024-08-24T06:59:13Z

but if we merge those in we'll need to wait for the flask-admin release (since there's at least one bug that was fixed in upstream after the current release)

We can wait for a while. No issues. How frequently do they release?

I tried to migrate the team posts, but it didn't work out. It is not listing the new Buku project discussion thread.
It has an option to enable discussions under Buku-dev and then probably we can migrate to that. However, I think it's convenient to use the project's discussion thread.

LeXofLeviafan · 2024-08-24T12:59:19Z

How frequently do they release?

I don't think they have a schedule, but the only remaining item they have on their pre-release TODO list is "Remove any existing DeprecationWarnings" 😅

As for team posts migration, I believe it's supposed to be migrated to either organization or a repo within it. And yes, we can use that but the point for migrating them is to at the very least have access to the posts archive in GitHub UI (as opposed to trying to navigate a single JSON file)… and I guess that would allow to discuss development plans before going public with them, as the team posts allowed (since I don't believe there's a way to make a public repo discussion non-public).

jarun · 2024-08-24T13:18:48Z

OK. Please ping me once they are through their release.

[jarun#753] pre-migration WebUI fixes

eafa9fe

LeXofLeviafan commented Aug 23, 2024

View reviewed changes

LeXofLeviafan commented Aug 24, 2024

View reviewed changes

jarun merged commit c114a01 into jarun:master Aug 24, 2024
1 check passed

LeXofLeviafan deleted the bootstrap3 branch August 24, 2024 17:46

github-actions bot locked and limited conversation to collaborators Jan 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[jarun#753] pre-migration WebUI fixes #761

[jarun#753] pre-migration WebUI fixes #761

LeXofLeviafan commented Aug 23, 2024

LeXofLeviafan Aug 23, 2024

LeXofLeviafan Aug 23, 2024

LeXofLeviafan Aug 23, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

LeXofLeviafan Aug 24, 2024

jarun commented Aug 24, 2024

LeXofLeviafan commented Aug 24, 2024

jarun commented Aug 24, 2024

LeXofLeviafan commented Aug 24, 2024

jarun commented Aug 24, 2024

		@@ -120,7 +119,6 @@ def shell_context():

		app.jinja_env.filters['netloc'] = lambda x: urlparse(x).netloc # pylint: disable=no-member

		Bootstrap(app)

		{{ buku.script('Chart.js') }}
		{% set NO_NETLOC = '\u200B(no netloc)\u200B' %}

[jarun#753] pre-migration WebUI fixes #761

[jarun#753] pre-migration WebUI fixes #761

Conversation

LeXofLeviafan commented Aug 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jarun commented Aug 24, 2024

LeXofLeviafan commented Aug 24, 2024

jarun commented Aug 24, 2024

LeXofLeviafan commented Aug 24, 2024

jarun commented Aug 24, 2024