elm-collage
elm-collage copied to clipboard
timjs
Simplistic size measurement of text causes overflow
Screenshot
Code
view : () -> Html Never
view _ =
Collage.Text.fromString "hamada"
|> Collage.rendered
|> Collage.Render.svg
<body>
<script>
var app = Elm.Main.init({ node: document.querySelector('body') })
</script>
</body>
Ellie
https://ellie-app.com/4mB5pLNhSJPa1
Description
Using the simplest way to render a string, sometimes some part of the beginning and end of it disappears. When using a string like foobarbaz, its rendered correctly. But when using, e.g., m, it is not.
Didn't go over all the characters that work and don't work correctly.
If you need any more details, please tell me.
I’m pretty sure this is due to the fact that width of text elements isn’t accurate: calculating the real width of the string requires a synchronous call to JavaScript (using Native code, which is forbidden in Elm 0.19), so instead the package gives an estimate based on the font size.
Since m is a relatively wide character the estimation is off.
The package determines the size of the svg based on the size of the elements within, so the width is a little too small.
One possible solution would be to use a monospace font.
Another would be to use Collage.Render.svgBox with manual width and height.
I’m guessing this is a known issue, but I don’t think there’s a way to fix it 😕
No problem. Thank you for your reply.
If I'd use Collage.Render.svgBox with manual width and height, how would I determine the width and height?
Yes, @danfishgold is completely right. In Elm 0.18, the library used a native call to the HTML canvas to measure the width of some text. In Elm 0.19 this is not allowed any more and I reverted to a very simplistic calculation using the font size and the string length. I'm open to any suggestions to make this work again.
Ideally, we would use some font library that measures text. Obviously, we can't make bindings to an existing JavaScript one any more, due to the restrictions in Elm 0.19.
Regrettably, I don't think ports are the way to go. If my understanding is correct, this would require every developer using this library to wire up to a port themselves. I think requiring every developer to do this is bad library design.
@timjs Thank you for your comprehensive reply.
For me, I wouldn't mind wiring ports, but my problem with ports is asynchronicity. I've tried to think of a design that would allow the user to just wire some ports but couldn't. Do you have one in mind?
@timjs Thank you for your comprehensive reply.
You're welcome.
For me, I wouldn't mind wiring ports, but my problem with ports is asynchronicity. I've tried to think of a design that would allow the user to just wire some ports but couldn't. Do you have one in mind?
Nope, when using ports it will always be async I guess.
Note: I've made a post on Elm Discourse in case anyone there has a work-around in mind.
Wonderful! Keep me posted if any solution comes up. I see decoding font files could be an option, but it is also async.
Forking the compiler works. Change this line to always return true: https://github.com/elm/compiler/blob/6c19c6e202984e2be98d6abe46b616751f712e94/compiler/src/Elm/Package.hs#L85 Rework to the package to use the new Kernel system instead of Native. I did it and it works.
@timjs I believe I figured out a way around this so that users would not have to use a modified compiler. It would require a couple of steps:
-
You put the width function in a separate module
-
You publish it normally with simplistic size measurements
-
You build the real text width package with a modified elm compiler. All you need is the artifacts.dat file.
-
Users replace the artifacts.dat file with the real one (I think this would just be one bash command)
-
Users can use the normal elm compiler based on the same version as the modified compiler to compile their programs.
If you are interested I can I perform further testing.
@timjs This command installs a prebuilt package for 0.19.1. I built it with a custom elm compiler. The elm file in the archive is just dummy code that returns 0, but the artifacts.dat was built from the other elm code which calls a kernel function.
mkdir -p ~/.elm/0.19.1/packages/2426021684/elm-text-width/1.0.1 && curl -L https://github.com/2426021684/elm-text-width/archive/1.0.1.tar.gz | tar -xz --strip 1 -C ~/.elm/0.19.1/packages/2426021684/elm-text-width/1.0.1
After running that command you can use 2426021684/elm-collage with normal elm 0.19.1 and everything works.
Nice workaround! Don’t have time to test it right away though, maybe somewhere next week.
How would a user of the package use and install this library? In the same way as above? If Elm itself won’t provide a convenient way to solve our problem, this is a good one for everybody that wants to do some extra work to get it
Sent with GitHawk
2426021684/elm-text-width is installed with the command I wrote. 2426021684/elm-collage can be installed the normal elm way after 2426021684/elm-text-width is installed.
Excellent! Can you maken a PR with te required code and add a description how to use it to the readme? I’ll review it
Sent with GitHawk
@timjs So this consists of three parts:
- Dummy text width package with simple calculation
- The prebuilt text width package with the kernel code
- Small changes to elm-collage
Only 3 makes sense as a PR to this repo. How do you want 1 and 2?
I take 3 is a small change to elm-collage instead of elm-diagrams? And can the dummy text width be built in in this library or do you really need a separate package? Otherwise we’ll add a note to the readme how to use the dummy package and your setup
Sent with GitHawk
While it is possible to put it all in elm-collage, it is better to put it in a separate package. Otherwise the examples don’t work and updating elm-collage becomes more difficult when using the kernel code. There needs to be two versions of the separate package. Do you want to maintain the second package? The dummy package would be install by elm-collage automatically. I could make the dummy fail to compile and require users to use the real one. I will most likely not have the time to maintain the package in the future.