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

Fix annotations in texts #534

Merged
merged 5 commits into from
Feb 12, 2025
Merged

Fix annotations in texts #534

merged 5 commits into from
Feb 12, 2025

Conversation

roewenstrunk
Copy link
Member

Description, Context and related Issue

This PR contains fixes for annotation icons in text view

Refs #460

How Has This Been Tested?

Tested with Open Faust data

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Overview

@roewenstrunk
Copy link
Member Author

@feuerbart , I created an issue in Open Faust (see https://git.uni-paderborn.de/kio/open-faust/-/issues/3). You should add some extra CSS to your edition specific file.

@roewenstrunk roewenstrunk added this to the 1.0.0 milestone Jan 22, 2025
@peterstadler peterstadler linked an issue Jan 23, 2025 that may be closed by this pull request
Copy link
Member

@peterstadler peterstadler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Activating the annotations now displays a tooltip symbol and hovering over it now triggers the tooltip to show, but

  1. you can't hide the annotation symbol anymore. Both the global as well as the per view "show/hide annotations" do not work
  2. extra vertical space is introduced in the second column
    Bildschirmfoto 2025-01-23 um 17 40 23

@roewenstrunk
Copy link
Member Author

The spacing in the Open Faust data is because of CSS issues in the edition, as I wrote earlier.
The "show/hide" of annotations is now fixed.
@peterstadler

@feuerbart
Copy link
Contributor

implemented the css, works fine with the OPERA-table-layout, thanks @roewenstrunk

Bildschirmfoto 2025-01-24 um 10 57 42

but when i switch to Edirom default layout, the annotation icon is placed below the line:

Bildschirmfoto 2025-01-24 um 10 58 19

seems to be connected to div { display: block; ... }.
playing around with display: inline-block; and defining some width, i could make it work, but i'm not sure if that is a good solution.

@roewenstrunk
Copy link
Member Author

Hi @feuerbart , I fixed the inline display and some width issues with visibility: hidden. Could you please test?
Hi @peterstadler , could you please accept the changes, you requested?

@feuerbart
Copy link
Contributor

Bildschirmfoto 2025-01-30 um 18 24 13

still breaking before the annotation icon. the preceding <div class="l"> has no block defined so the browser defines it as display: block; what breaks after the element. setting that manually to display: inline-block; fixes the icons position but will break the lines layout.
have done some research but not yet found a solution that will not break after the div for other divs but not for spans.

@feuerbart
Copy link
Contributor

i just tested all the options for display with divs, none is a solution.
but moving the <span class="note">... inside the <div class="l">... did it. is there a reason why it is not already there?
the onmouseovers won't work like that but that could be fixed, right?

@daniel-jettka
Copy link
Contributor

kind ping to @roewenstrunk @peterstadler

@daniel-jettka
Copy link
Contributor

Kristin and I tested and on my machine Linux with Firefox and Chrome it looks like this:

image

Should be right like this, correct? @feuerbart @peterstadler And if yes, could you update your review?

@feuerbart
Copy link
Contributor

feuerbart commented Feb 11, 2025

if you deployed it from git, then that is as it should be. but there is a custom transformation implemented (some OPERA-legacy reason...) that uses a <table> as layout (the only issue here ist the slight shift in height).

if you remove the entry in the prefs and the transformationin edition_text, then the annotation icon will move below the line.

could it be a solution to use the table instead of divs? (also haven't tested line numbers with divs so far)

@daniel-jettka
Copy link
Contributor

Hi @feuerbart Thanks for looking into this again!

What exactly do you mean by "if you deployed it from git"? Do you mean the Edirom or the edition,and which branch?

And is your comment missing sth? It seems like there is a broken sentence... :-)

"...) that uses a

as layout ..."

@feuerbart
Copy link
Contributor

yeah, sorry, there is something crucial missing in that sentence, probably some brainafk selection of that part and pressing enter without realizing it...

complete and rephrasedit should be that:
if you build the faust-package from git main branch (edirom version: branch fix-annotations-in-texts), then it looks like your screenshot. but there is a custom transformation implemented (some OPERA-legacy reason...) that uses a modified Transformation.xls (see edition/edition_text.xml#L1) and modified xql/getText.xql (see prefs.xml, prefs/prefs.xml#L42) which implement a <table>-layout instead of the edirom default <div>layout.
if you remove them both (aka default settings) it should look like this: #534 (comment) because of the lines display: block; that will break after itself and push the icon to the next line. changing display to e.g. inline-block would fix that, but mess up the lines. i haven't found a solution for that breaking with <div> yet. as i have tested all possible values for display, i could not make one work to show the desired result (that is breaking after the lines <div> but not if there is a icons <div> following). could be that there is no solution for that.

i just removed the custom settings in the openedirom exist: https://openedirom.lilienteich.de/exist/apps/Edirom-Online/index.html?uri=xmldb:exist:///db/apps/edirom/open-faust/content/edition/edition_text.xml
if you enable the annotation icons, you can see the problem (first annotation in l. 48: "Trüb’ durch die Fensterscheiben bricht!").

the OPERA table-layout works quite fine, the only issue here ist the slight shift of the lines height when you enable the icons. see here: #534 (comment), the space above l. 48 is slightly bigger than the other lines.

note: i just found out what messed up the other comment: the linebreak was a <table> that was not parsed as text but rendered as html and as it had no content, it was just blank space...
just edited that comment to be displayed correctly...

Copy link
Member

@peterstadler peterstadler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roewenstrunk convincingly presented the fixes during today's Community meeting

@daniel-jettka daniel-jettka merged commit bb7a180 into develop Feb 12, 2025
5 checks passed
@daniel-jettka daniel-jettka deleted the fix-annotations-in-texts branch February 12, 2025 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[BUG] annotations display in text edition
4 participants