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

[BUG] annotations display in text edition #460

Open
AndreasMunzmay opened this issue Nov 6, 2024 · 4 comments · May be fixed by #534
Open

[BUG] annotations display in text edition #460

AndreasMunzmay opened this issue Nov 6, 2024 · 4 comments · May be fixed by #534
Assignees
Milestone

Comments

@AndreasMunzmay
Copy link

AndreasMunzmay commented Nov 6, 2024

Describe the problem
neither annotation buttons nor annotation preview appears in the text edition

To Reproduce:

  1. Go to https://openedirom.lilienteich.de/exist/apps/Edirom-Online/index.html?edition=edition-LindP&lang=en
  2. Open Edition>Text
  3. Show Critical Notes

Expected Behaviour:
Annotations buttons in the right column
Annotation preview via mouse-over

Data
https://git.uni-paderborn.de/kio/open-faust/-/tree/main/content/sources?ref_type=heads

Additional context
s within the TEI file are placeholders. ALL critical notes of the edition are stored in one common file (critical_notes.xml). This is necessary because there's only one critical apparatus for the edition as a whole. Thus, a critical note can refer to the music view, to the text view, or to both views of the edition - and of course to up to five out of the seven sources.

@AndreasMunzmay AndreasMunzmay changed the title [BUG] [BUG] annotations display in text edition Nov 6, 2024
@feuerbart feuerbart self-assigned this Jan 15, 2025
@feuerbart
Copy link
Contributor

@peterstadler peterstadler added this to the 1.0.0 milestone Jan 16, 2025
@peterstadler peterstadler linked a pull request Jan 16, 2025 that will close this issue
@feuerbart
Copy link
Contributor

we have some OPERA-legacy regarding the formatting of the text edition, that i disabled for now. so it should be all edirom defaults.

https://openedirom.lilienteich.de/exist/apps/Edirom-Online/index.html?uri=xmldb:exist:///db/apps/edirom/open-faust/content/edition/edition_text.xml
first text annotation is in l. 48 Trüb’ durch die Fensterscheiben bricht! (third paragraph)
in the HTML the annotations exist, but css says display: none;
also the annotations button in the menu is displayed in dark grey, seems to be disabled. probably text annotations are disabled somewhere globally? (side note: same annotations in music sources work)

peterstadler added a commit that referenced this issue Jan 16, 2025
## Description, Context and related Issue

There surfaced two issues when testing with the Open-Faust data which
might be connected to #460.
1. JSONification of the `local:getAnnotations` function was not
complete. The function returned a map object `plist` as string, causing
some Javascript function to fail.
2. `teitext:getLabel#2` would return a sequence of strings (under some
circumstances) which was not allowed by the return value of the function
signature.

<!--- This project only accepts pull requests related to open issues.
Please link to the issue here: -->
Refs #460


## Types of changes
- Bug fix (non-breaking change which fixes an issue)
- Improvement
- Refactoring

## Overview
- I have performed a self-review of my code, according to the [style
guide](https://github.com/Edirom/Edirom-Online/blob/develop/STYLE-GUIDE.md)
- I have read the
[CONTRIBUTING](https://github.com/Edirom/Edirom-Online/blob/develop/CONTRIBUTING.md)
document.
- All new and existing tests passed.
@peterstadler
Copy link
Member

While #532 fixes some issues regarding text annotations it does not address this issue completely.
In addition to @feuerbart 's last comment about the CSS issues there's yet another bug: when checking "show annotations" globally from the bottom menu and then opening the text view it will fail to process the annotations correctly.

var target = me.el.getById(me.id + '_' + targetId);
target will be null – most probably the text content is not already loaded and the getById fails to find anything. showAnnotations would need to wait for the Ajax request to getText.xql to succeed first.

@peterstadler peterstadler moved this to In Progress in Edirom Development Jan 16, 2025
@feuerbart
Copy link
Contributor

clicking show/hide annotations via the menu works mostly as intended (at least i think so):
getAnnotationsInText.xql has proper output:

[ {
  "id" : "opera_annot_45543371-02c6-46cc-a818-dbb73aa39eb8",
  "plist" : [ {
    "id" : "opera_annot_45543371-02c6-46cc-a818-dbb73aa39eb8__note_l-48"
  } ],
  "svgList" : [ ],
  "fn" : "loadLink(\"xmldb:exist:///db/apps/edirom/open-faust/content/critical_report/critical_notes.xml#opera_annot_45543371-02c6-46cc-a818-dbb73aa39eb8\")",
  "uri" : "xmldb:exist:///db/apps/edirom/open-faust/content/critical_report/critical_notes.xml#opera_annot_45543371-02c6-46cc-a818-dbb73aa39eb8",
  "priority" : "ediromAnnotPrio1",
  "categories" : "ediromAnnotCategory_Text"
}, {

the html looks good to me:

<span class="note" id="textView-1063_note_l-48">
  <span class="noteLabel">Note: </span>
  <span id="textView-1063_opera_annot_45543371-02c6-46cc-a818-dbb73aa39eb8__note_l-48" class="annotation ediromAnnotCategory_Text ediromAnnotPrio1 opera_annot_45543371-02c6-46cc-a818-dbb73aa39eb8" data-edirom-annot-id="opera_annot_45543371-02c6-46cc-a818-dbb73aa39eb8"></span>
</span>

and if if add some text to that last span and deactivate the css display: none; of the outer span, it even shows the added text in the text edition and opens the annotations tooltip with the correct preview:

Image

Image

(the Note:-content of the first inner span won't be shown because it has an own display: none;)

@roewenstrunk roewenstrunk linked a pull request Jan 22, 2025 that will close this issue
@peterstadler peterstadler linked a pull request Jan 23, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

3 participants