CfW: add a test for Text Baseline #1314
Conversation
| modifier = Modifier.alignByBaseline() | ||
| .onGloballyPositioned { | ||
| headingOnPositioned.trySend(it[FirstBaseline]) | ||
| println("The heading alignment line is ${it[FirstBaseline]}\n") |
There was a problem hiding this comment.
should we keep this println in test?
| assertTrue(headingAlignment > 0) | ||
| assertTrue(subtitleAlignment > 0) | ||
| assertTrue(headingAlignment > subtitleAlignment) | ||
| } |
There was a problem hiding this comment.
Two questions\suggestions here. First one (more of a suggestion than question) if we do believe that the actual tests would check for something being bigger than specific value, then we can introduce a specialised assertion - so that we'll immediately see which of three has failed and with was exactly value. Something like assertIsGreater
Second (more of a question rather a suggestion) - is it enough? I mean knowng that something is greater than zero? Can we check for a specific value, even with tolerance or something?
There was a problem hiding this comment.
Can we check for a specific value, even with tolerance or something?
I noticed that the actual value on CI is a bit smaller than on my machine. And it's okay. The actual value depends on the screen resolution (and density too), I guess.
I think that pre-calculating the exact expected value is possible, but it would likely contain some hardcode anyway. And then if, for example, the default font changes, the expected value will need to be updated.
Given it's something device specific, I decided to simplify the check to avoid surprising test failures on some machines.
assertIsGreater - ACK
| import kotlinx.browser.document | ||
| import org.w3c.dom.HTMLCanvasElement | ||
|
|
||
| internal interface OnCanvasTests { |
There was a problem hiding this comment.
A small doc won't harm - like this interfaces is supposed to be used in such and such cases. I mean, it can be obvious to you and to me but not necessarily for our coworkers, current of future )
eymar
force-pushed
the
ok/add_text_for_text_baseline
branch
from
a02d87a
to
6fb7e9d
Compare
The issue JetBrains/compose-multiplatform#4078 was fixed earlier. Probably in skiko: JetBrains/skiko#846