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

KB: Improved Links (OX-11128) #2024

Merged
merged 13 commits into from
Jul 23, 2024
Merged

Conversation

jakobvogel
Copy link
Member

Description

Improves the <k:ref> and <k:link> tags:

  • Custom label and icon for <k:ref>
  • Internationalised fallback with tooltip and visual marking when referencing an inaccessible KBA
  • Labels can be provided via attribute or as nested DOM-tree (as one would do for <a>)
  • name attribute of <k:link> is deprecated in favour of this label, due to misleading naming.
  • Links now always show a label
  • No excessive whitespace interfering with punctuation characters

<k:link>

Ich bin <k:link link="https://www.google.de"/>.
Ich bin <k:link link="https://www.google.de">L<b>i</b>nk1</k:link>.
Ich bin <k:link link="https://www.google.de" name="Link2"></k:link>.
Ich bin <k:link link="https://www.google.de" name="Link3"/>.
Ich bin <k:link link="https://www.google.de" label="Link4"></k:link>.
Ich bin <k:link link="https://www.google.de" label="Link5"/>.
Ich bin <k:link icon="" link="https://www.google.de"/>.
Ich bin <k:link icon="" link="https://www.google.de">Link1</k:link>.
Ich bin <k:link icon="" link="https://www.google.de" name="Link2"></k:link>.
Ich bin <k:link icon="" link="https://www.google.de" name="Link3"/>.
Ich bin <k:link icon="" link="https://www.google.de" label="Link4"></k:link>.
Ich bin <k:link icon="" link="https://www.google.de" label="Link5"/>.
Ich bin <k:link icon="fa-solid fa-money-bill-wave" link="https://www.google.de"/>.
Ich bin <k:link icon="fa-solid fa-money-bill-wave" link="https://www.google.de">Link1</k:link>.
Ich bin <k:link icon="fa-solid fa-money-bill-wave" link="https://www.google.de" name="Link2"></k:link>.
Ich bin <k:link icon="fa-solid fa-money-bill-wave" link="https://www.google.de" name="Link3"/>.
Ich bin <k:link icon="fa-solid fa-money-bill-wave" link="https://www.google.de" label="Link4"></k:link>.
Ich bin <k:link icon="fa-solid fa-money-bill-wave" link="https://www.google.de" label="Link5"/>.
Screenshot 2024-07-19 at 16 44 33

<k:ref>

Ich bin <k:ref code="ZZZZZ"/>.
Ich bin <k:ref code="FSZ13"/>.
Ich bin <k:ref code="ZZZZZ" icon="fa-solid fa-money-bill-wave"/>.
Ich bin <k:ref code="FSZ13" icon="fa-solid fa-money-bill-wave"/>.
Ich bin <k:ref code="ZZZZZ">Link1</k:ref>.
Ich bin <k:ref code="FSZ13">Link2</k:ref>.
Ich bin <k:ref code="ZZZZZ" label="Link3"></k:ref>.
Ich bin <k:ref code="FSZ13" label="Link4"></k:ref>.
Ich bin <k:ref code="ZZZZZ" label="Link5"/>.
Ich bin <k:ref code="FSZ13" label="Link6"/>.
Screenshot 2024-07-19 at 16 44 07 Screenshot 2024-07-19 at 16 44 22

Additional Notes

  • This PR fixes or works on following ticket(s): OX-11128

Checklist

  • Code change has been tested and works locally
  • Code was formatted via IntelliJ and follows SonarLint & best practices

First, using the body block (i.e. nested elements) to pass the caption of a link feels more HTML-ish. Second, it is probably not well-defined what the `name` of a link is supposed to be. Therefore, this commit introduces a new attribute `label` that defaults to the rendered content of the element. The old `name` attribute is deprecated and used as fallback only.

OX-11128
Links without labels render only as icons with excessive whitespace. This does not look good, does not make much sense, and is probably also not good for screen readers.

OX-11128
Like that, we will be able to have reference links in KBAs that follow the flow of the text without resorting to normal links. The new label can be provided either as an attribute, or via the child content of the element.

OX-11128
The label is now translated to the user's language, shows an error icon and is rendered in red. In addition, a tooltip provides context information about the error.

OX-11128
This whitespace may lead to spaces appearing in front of punctuation characters where it is plainly wrong.

OX-11128
@jakobvogel jakobvogel added the 🧬 Enhancement Contains new features label Jul 19, 2024
@sabieber
Copy link
Member

Mhhhh I don't know whether I would prefer a more "graceful"/silent failing when the user has no access to the underlying KBA. Wouldn't it be better to just schow the label as plain text instead. Is there any benefit for the user to see that there could have been some link?

@jakobvogel
Copy link
Member Author

Mhhhh I don't know whether I would prefer a more "graceful"/silent failing when the user has no access to the underlying KBA. Wouldn't it be better to just schow the label as plain text instead. Is there any benefit for the user to see that there could have been some link?

There are several possible scenarios where this question matters:

  • A legitimate link that should be available to some users with certain permissions, but not to others. In this case, the graceful option appears to be the optimal one.
  • An editing error where a non-existent or inaccessible link is placed into a KBA. In this case, the error should be visible, such that it can be fixed.

Considering that scireum editors have privileged access, the error case appears to be of relevant likelihood, and marking the error appears to be prudent such that customers can complain, as they can still tell that there should have been a link.

For the first case, equivalent behaviour could be reached via use of <t:permission>. This would have the huge advantage that this intended behaviour is obvious in the code, and would not rely on ”magical“ behaviour of <k:ref>

(Originally discussed in person during the OX Daily on July 23)

@jakobvogel jakobvogel merged commit 3196c48 into develop Jul 23, 2024
3 checks passed
@jakobvogel jakobvogel deleted the feature/jvo/OX-11128-KBA-Improvements branch July 23, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧬 Enhancement Contains new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants