Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ feat: allow inverting previous/next article links #261

Merged
merged 10 commits into from
Jan 28, 2024

Conversation

ZzMzaw
Copy link
Contributor

@ZzMzaw ZzMzaw commented Jan 28, 2024

Summary

3 changes are introduced:

  1. Replace angle brackets (/) by arrows (/) in article links
  2. Add the capability to reverse the order of article links (from next / previous to previous / next)
  3. Introduce labels in front of article links

Related issue

#259

Changes

At lines L.137-138 of templates/page.html, I had to retrieve labels in a variable because I was not able to concatenate a macro and a string.
{% set article_links_left_side_title = macros_translate::translate(key="prev", default="Prev", language_strings=language_strings) ~ " - " ~ page.higher.title %} fails on my side.
In the end, it can make sense to retrieve them once as they are used several time in later code.

I was not able to make lines L.139-157 of templates/page.html less verbose.
I tried several things but I cannot create structures (kind of json object) directly in the template (or at least, I was not able to code it).
In addition, I was not able to figure out from the code already in this page if I had to remove whitespace before/after statements so I kept them.
Feel free to tell me if it doesn't suit.

I updated images in "Mastering tabi Settings: A Comprehensive Guide" article but I took screenshots manually. Even if both light and dark images are strictly identical (same position and size), they may slightly differ from what you usually do. Feel free to update them to your liking.

I didn't tried to do translations neither in Español nor in Català this time as they would be too much approximate and I prefer to rely on you than on Google translate if you're ok.

Regarding the labels, I just tried to propose something which is language agnostic (e.g. : may need space before depending on the language) and make sure the entire length (label + title) will be truncated to 100 characters as label length will change depending on the language.
Please don't hesitate to propose anything else.

Last but not least, I am not sure how I should verify the accessibility of such changes so I preferred not to tick the box.

Accessibility

I just added ARIA link attribute to navigation links on top of what already in place (I would say semantic HTML and Clour contrast).
Even if I was not able to test neither keyboard navigation nor voiceover or screen reader compatibility, I would say there should not be impacted by this change.

Screenshots

Before:
before

After:
after

Type of change

  • Bug fix (fixes an issue without altering functionality)
  • New feature (adds non-breaking functionality)
  • Breaking change (alters existing functionality)
  • UI/UX improvement (enhances user interface without altering functionality)
  • Refactor (improves code quality without altering functionality)
  • Documentation update
  • Other (please describe below)

Checklist

  • I have verified the accessibility of my changes
  • I have tested all possible scenarios for this change
  • I have updated theme.toml with a sane default for the feature
  • I have made corresponding changes to the documentation:
    • Updated config.toml comments
    • Updated theme.toml comments
    • Updated "Mastering tabi" post in English
    • (Optional) Updated "Mastering tabi" post in Spanish
    • (Optional) Updated "Mastering tabi" post in Catalan

Default order is next on the left and previous on the right.
Reverse order is previous on the left and next on the right.

Documentation update limited to english version only.
@ZzMzaw ZzMzaw requested a review from welpo as a code owner January 28, 2024 09:07
@welpo
Copy link
Owner

welpo commented Jan 28, 2024

Fantastic job!

Everything looks good to me; thanks for being so thorough.

I've just made a small rephrase on the docs. I will translate to Spanish / Catalan and merge.

Thanks again!

@welpo
Copy link
Owner

welpo commented Jan 28, 2024

Translation done. Before merging let's discuss the Next/Prev prefix position/look.

@ZzMzaw
Copy link
Contributor Author

ZzMzaw commented Jan 28, 2024

Just added the code which allowed me to generate captures in the issue.

I tried to use aria-describedby tag to link the link to the article title but I am absolutely not sure it is the way it is supposed to be used.

I also have a doubt on the font-weight to the title in terms of visibility. So far, I took the same as the one used for metadata of the article.

@welpo welpo changed the title Feat/previous next article links order ✨ feat: allow inverting previous/next article links Jan 28, 2024
@welpo welpo merged commit b011f58 into welpo:main Jan 28, 2024
@ZzMzaw ZzMzaw deleted the feat/previous-next-article-links-order branch January 30, 2024 21:10
Smtbook pushed a commit to Smtbook/zola-theme-tabi that referenced this pull request Feb 29, 2024
- Adds next/previous labels for clarity
- Truncated long titles get "…" appended

Co-authored-by: welpo <welpo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants