~tsileo/microblog.pub-devel

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH] Use <details> element for sensitive text

Details
Message ID
<20221125065657.582027-1-jane@janeirl.dev>
DKIM signature
missing
Download raw message
Patch: +18 -14
The sensitive text feature was implemented with <label> and hidden
checkbox <input> elements. There were two issues with this
implementation:
1. The user couldn't navigate to the "show/hide more" button using
   keyboard.
2. The label indicates two actions at the same time ("show/hide more"),
   making it unclear what the function of the checkbox was and what the
   current show/collapse state was.

As it is generally preferrable to use built-in HTML elements for the
best semantic, this commit moves to use the <details> and <summary>
elements for the sensitive text feature. The browser will open/collapse
the content in <details> automatically when the user clicks on the
<summary>, and keyboard navigation support is built-in.

This commit also changes the button to display "show more" or "show
less" depending on the state for visual clarity. This button is hidden
from the accessibility tree using `aria-label="false"`, as the <details>
element already exposes its state to the tree and we want to avoid
duplicated information.

A few caveats:
* The "show/hide sensitive content" button for sensitive attachments
  hasn't been changed yet as I'd like to get more feedback about the new
  implementation.
* As the summary/content warning text itself is also part of the
  <summary> tag, the user can now also click on them to toggle the
  visibility of the sensitive text. This may not be desirable as the
  current interface does not make it clear that this could happen; the
  user may try to select some text from the summary and be surprised
  by the sensitive text being expanded. One way to improve this would
  be to add an event listener to the summary text and call
  `preventDefault`, but this would introduce JavaScript code.
---
 app/scss/main.scss       | 17 ++++++++++-------
 app/templates/utils.html | 15 ++++++++-------
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/app/scss/main.scss b/app/scss/main.scss
index 8aa23e9..75e8e90 100644
--- a/app/scss/main.scss
+++ b/app/scss/main.scss
@@ -51,17 +51,20 @@ $code-highlight-background: #f0f0f0;
  .p-summary {
    display: inline-block;
  }
  label {
  .show-more-btn {
    margin-left: 5px;
  }
  .show-more-state {
    display: none;
  summary {
    display: inline-block;
  }
  .show-more-state ~ .obj-content {
    margin-top: 0;
  summary::-webkit-details-marker {
    display: none
  }
  .show-more-state:checked ~ .obj-content {
    display: none;
  &:not([open]) .show-more-btn::after {
    content: 'show more';
  }
  &[open] .show-more-btn::after {
    content: 'show less';
  }
}
.sensitive-attachment {
diff --git a/app/templates/utils.html b/app/templates/utils.html
index 2cf460a..1e4ae4a 100644
--- a/app/templates/utils.html
+++ b/app/templates/utils.html
@@ -549,12 +549,13 @@
    {% endif %}

    {% if object.summary %}
    <div class="show-more-wrapper">
        <div class="p-summary">
            <p>{{ object.summary | clean_html(object) | safe }}</p>
        </div>
        <label for="show-more-{{ object.permalink_id }}" class="show-more-btn">show/hide more</label>
        <input class="show-more-state" type="checkbox" aria-hidden="true" id="show-more-{{ object.permalink_id }}" checked>
    <details class="show-more-wrapper">
        <summary>
            <div class="p-summary">
                <p>{{ object.summary | clean_html(object) | safe }}</p>
            </div>
            <span class="show-more-btn" aria-hidden="true"></span>
        </summary>
    {% endif %}
        <div class="obj-content">
        <div class="e-content">
@@ -615,7 +616,7 @@

        </div>
    {% if object.summary %}
    </div>
    </details>
    {% endif %}

    <div class="activity-attachment">
-- 
2.38.1
Details
Message ID
<8bd69aee-41b7-4cb6-a56b-ed559dcb4cb6@app.fastmail.com>
In-Reply-To
<20221125065657.582027-1-jane@janeirl.dev> (view parent)
DKIM signature
missing
Download raw message
Hey,

Thank you for this patch, it makes sense! I may add a tiny bit of JS to disable expanding when clicking on the text, but I think we can live with it.

I just applied it.

Thank you!
Reply to thread Export thread (mbox)