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

Proposal to rework drama to not use tables #780

Open
robinwhittleton opened this issue Jan 1, 2025 · 15 comments
Open

Proposal to rework drama to not use tables #780

robinwhittleton opened this issue Jan 1, 2025 · 15 comments

Comments

@robinwhittleton
Copy link
Member

We currently use tables to lay out drama, but this is problematic:

  1. Apple Books has a zoomable table viewer mode that shows up when you click on the text, and breaks the reading experience.
  2. Screen readers typically have a special table viewing mode that moves the user into a row / column navigation metaphor.

We’ve attempted to work past the first problem in the past with CSS that disallows clicking on tables, but this has the side effects of breaking selection and dictionary lookup.

First attempt to properly fix this

I tried adding role="presentation" to the drama tables. This is the recommended fix for problem 2, and partially resolves 1, but it’s really flaky: some parts of the table no longer produce a viewer mode, but not others. I attempted to add role="presentation" additionally to the table rows with no luck.

Second proposal

This would be considerably more work, but I think we should evaluate using display: table with classed divs instead. To test, I converted The Libation Bearers, replacing <table> with <div class="drama-table">, <tr> with <div class="drama-row">, and <td> with <div class="drama-cell">. I then added appropriate display: table / display: table-row / display: table-cell to local.css.

A screenshot of the proposed markup.

Testing in Apple Books (with the viewer suppression hack removed), this looks fine:

A screenshot of the first verse showing the character’s name on the left and the text on the right.

I don’t have a physical Kindle, but I tested loading the same epub into Kindle Previewer, and it looks fine there too:

A screenshot of the first verse showing the character’s name on the left and the text on the right.

Work to be done

If we decide to go ahead with this, then the good thing is that it doesn’t need to be a big bang release. The proposed timeline would be:

  1. Test the proposal with physical Kindle, more ereaders, and more dramas, to find latent compatibility issues.
  2. Rework the manual, and any supporting how-to guides with the new approach.
  3. Produce a script that would do the majority of the rework automatically against a book.
  4. Rework lint to remove the drama workarounds from table checks.
  5. List all productions with drama, and split them up among volunteers to each do the rework, fix outstanding issues the script didn’t cover, and test.
  6. Once all the existing books have been converted, remove the Apple Books compatibility hack from the CSS.

Question

Worth it?

@acabal
Copy link
Member

acabal commented Jan 2, 2025

Good work. I think role="presentation" would be the best way forward as it is technically correct while avoiding having to redo the whole corpus.

Using <div>s might also work... our current approach is, after all, semantically incorrect, and we only do it because it's the best solution for our appearance requirements given old bad renderers.

We could even avoid the extra classes by selecting .drama > div{ /* table row */ } and .drama > div > div{ /* table cell */ }.

However I suspect that display: table* isn't going to work on ADE which is still a significant portion of readers. You can test by uploading a plain epub (not kepub) to a Kobo, which will trigger the ADE renderer. You may also still be able to download a desktop version of the ADE reader to test? Haven't looked in a while.

In any case, any solution we select has to work in ADE.

@acabal
Copy link
Member

acabal commented Jan 2, 2025

@acabal
Copy link
Member

acabal commented Jan 3, 2025

I think even if none of this shakes out, we should at least add role="presentation" as the standard because that is semantically correct.

@robinwhittleton
Copy link
Member Author

display: table seems to work nicely in ADE 4.5.12 on my mac:

Screenshot 2025-01-04 at 21 52 04

Whatever version of ADE is installed on my Kobo originally looked like a mess (although the display: table layout worked) until as a test I copied over the current release version of The Libation Bearers and got exactly the same result. So we know that this alternative approach isn’t specifically broken at least. Here’s a photo of my custom build:

IMG_1683 Medium

@robinwhittleton
Copy link
Member Author

I guess the next step would be to test on a physical Kindle? If you (or anyone else reading) has one I’ve attached my reworked display: table AZW3 (zipped to keep GitHub happy). We’re looking for horizontal alignment of persona to their text, as per the screenshots in this issue.

aeschylus_the-libation-bearers_gilbert-murray.azw3.zip

@acabal
Copy link
Member

acabal commented Jan 5, 2025

Can you send the compatible epub version that I can try sending to my Kobo?

@robinwhittleton
Copy link
Member Author

Certainly. Here’s the compatible and Kobo variants.

@alerque
Copy link

alerque commented Jan 6, 2025

Here are some screenshots of assorted pages from an aging Kindle Paperwhite (Kindle firmware 5.6.1.1):

https://photos.app.goo.gl/4C85aEA4MSxUsGj97

If any other pages or perhaps text size views would be useful just let me know. This is just at the text size and preferences I had from last reading a book.

@acabal
Copy link
Member

acabal commented Jan 7, 2025

OK, I tried it on my very old Kindle Voyage and Kobo Aura One and they both look good.

Lastly I think we should try the renderers on some of the bigger ereading apps. Google Books, Kindle app (not the same renderer as the eink Kindle), Nook app, Moon+, FBreader. (FBreader I recall has an especially bad renderer so I'm not super concerned if it doesn't look perfect in that one.)

@robinwhittleton
Copy link
Member Author

Google Play
table-google-play

Kindle macOS
table-kindle-macos

Thorium
table-thorium

FBreader
table-fbreader

Moon+ seems to be Android-only, and I don’t have an Android device available currently. And Nook is US-only, it won’t let me download from the Swedish app store.

@acabal
Copy link
Member

acabal commented Jan 9, 2025

OK looks good.

The next question is, how would we handle the .together class, e.g. here?

@EmmaSweeney do you have any thoughts on this proposal?

@robinwhittleton
Copy link
Member Author

In Apple Books it seems to work fine. Borders, padding and vertical-align should work in anything that supports display:table. Screenshot from Books with my test build:

Screenshot 2025-01-10 at 16 49 28

@acabal
Copy link
Member

acabal commented Jan 10, 2025

But what about multiple speakers together in one dialog? We used rowspan before. We'll have to think of something else.

@robinwhittleton
Copy link
Member Author

Right, yes. Let me have a think about how to handle that.

In the mean time, I’ve started a branch with a convert-drama command. The goal here isn’t that this will be merged in, but it should make it easier to update the corpus by automating the conversion process before checks. At the moment the command just fixes standard drama markup, CSS and .together rowspans will follow.

@EmmaSweeney
Copy link

I'm happy that we are improving the reading experience for our plays! I don't think there is anything besides the together class that could cause any problems when switching to this format. Have you tested this on drama dialog in an epigraph (example)?

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

No branches or pull requests

4 participants