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

Add background and foreground color on span tests #105

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

btsimonh
Copy link
Contributor

color001: foreground color change when background present on span - illustrates how/why to keep P on a single line.
color002: background color change on span
color003: 2nd background color change on span - illustrates avoiding a gap.
color004: background color change on span when over P with background. Note doubled opacity.

color001: foreground color change when background present on span - illustrates how/why to keep P on a single line.
color002: background color change on span
color003: 2nd background color change on span - illustrates avoiding a gap.
color004: background color change on span when over P with background.  Note doubled opacity.
@palemieux
Copy link
Contributor

Is the following what you expect for color001?

image

@palemieux
Copy link
Contributor

@btsimonh Are these tests specific to IMSC 1.1, or would they also work on IMSC 1.0.1?

@palemieux
Copy link
Contributor

Is this the expected result for color002?

image

@palemieux
Copy link
Contributor

Is this the expected result for color003?

image

@palemieux
Copy link
Contributor

@btsimonh Have you validated all the proposed tests? color003 has no ID/IDREF binding for IDREF 'p1'.

@palemieux
Copy link
Contributor

palemieux commented Oct 11, 2022

Is this expected render for color004?

image

[EDIT: replaced render with render from the latest version of imscJS]

@palemieux palemieux changed the title add color tests: Add background and foreground color on span tests Oct 11, 2022
@palemieux palemieux self-requested a review October 11, 2022 17:00
Copy link
Contributor

@palemieux palemieux left a comment

Choose a reason for hiding this comment

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

The files should be valid and the submitter should verify that the rendering matches their expectations.

@btsimonh
Copy link
Contributor Author

answers:
Presentation of your images is as expected for 1-3.
Except color004. I would not expect the darker box before 'P'. It does not do that for me?
Have remove refs to style P1.
s

@btsimonh
Copy link
Contributor Author

Are these tests specific to IMSC 1.1, or would they also work on IMSC 1.0.1

I believe they should be good on 1.0.1?

@palemieux
Copy link
Contributor

I believe they should be good on 1.0.1?

Any reason not to move them to IMSC1 directory then?

@palemieux
Copy link
Contributor

Except color004. I would not expect the darker box before 'P'. It does not do that for me?

Oh. I used an old version of the imscJS renderer. The most recent version does not have the artifact. I have updated #105 (comment)

@btsimonh
Copy link
Contributor Author

Any reason not to move them to IMSC1 directory then?

depends on if the folders repersent.

Personally, I would test only against the latest tests, unless it explicitly said to test against all 3 sets?

@nigelmegitt
Copy link
Contributor

I would test only against the latest tests, unless it explicitly said to test against all 3 sets?

The tests are layered, as is the functionality required in the different versions of the IMSC specification. Tests that apply to all versions are in the IMSC1 directory, ones that apply to functionality introduced in 1.1 would be in the 1.1 directory, likewise with 1.2 etc. So a 1.2 implementation should be tested against the tests in the 1, 1.1 and 1.2 directories.

@btsimonh
Copy link
Contributor Author

Let me review the tests in the imsc1 folder before re-proposing any changes if required....

"The tests are layered, as is the functionality required in the different versions of the IMSC specification. Tests that apply to all versions are in the IMSC1 directory, ones that apply to functionality introduced in 1.1 would be in the 1.1 directory, likewise with 1.2 etc. So a 1.2 implementation should be tested against the tests in the 1, 1.1 and 1.2 directories."
@nigelmegitt - please can we add this text to the readme for the repo - just to make it explicit.

@nigelmegitt
Copy link
Contributor

please can we add this text to the readme for the repo

Raised #106 for this.

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.

3 participants