readium / readium-css

🌈 A set of reference stylesheets for EPUB Reading Systems, starting with Readium Mobile
https://readium.org/readium-css/
BSD 3-Clause "New" or "Revised" License
90 stars 20 forks source link

Make the before-CSS hide footnote(s) by default. #83

Closed tooolbox closed 3 weeks ago

tooolbox commented 4 years ago

Please check if the PR fulfills these requirements:

What kind of change does this PR introduce? (Bug fix, feature, docs update, other)

Feature.

What is the current behaviour? (You can also link to an open issue here)

Elements matching aside[epub|type^="footnote"] are visible in the flow of text.

What is the new behaviour?

Based on the fact that these elements are meant to be shown via popups, they are now hidden in the flow of text. The reading system will need to display them in response to the user tapping a hyperlink that would otherwise navigate to one of these elements (where that hyperlink matches a[epub|type="noteref"]).

Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)

Yes, books that displayed these <aside> elements in the flow of text will no longer display them by default.

Since this new CSS rule is in the "before" CSS, publication authors can choose to override it if desired.

Otherwise, the app using Readium CSS will need to support the detection of footnotes and display them in a popup. Note that R2-Navigator for Android already supports this.

Other information:

The same rules could be applied to endnote(s) and/or rearnote(s) but I have chosen not to as it makes more sense to include them in the flow of text, compared to footnotes, as they would fall at the end of a section (chapter, likely) and therefore not interrupt the reader in the same way as a footnote.

As I write the section above on breaking changes, and with https://github.com/readium/swift-toolkit/issues/139 in mind, I come to these thoughts:

Let's discuss!

tooolbox commented 4 years ago

As a note, the Readium ICLA says to email it to secretary@readium.org but my email to there just bounced. :confused:

mickael-menu commented 4 years ago

ReadiumCSS could be used outside of our navigator implementation, so personally I would vote to have this feature disabled by default, and enabled by our navigator.

As a note, the Readium ICLA says to email it to secretary@readium.org but my email to there just bounced. 😕

I'm not sure this is necessary anymore, but @rkwright and @llemeurfr can probably help you out for this.

llemeurfr commented 4 years ago

I'm not the secretary of the Readium Foundation :-); let's forget the ICLA for now, I'm sure most developers of this community didn't sign it.

JayPanoz commented 4 years ago

Thanks. Indeed this is tied to ongoing discussions in Swift so it’s likely to evolve, but at first glance:

I think hiding by default is for once something that would be too much opinionated for ReadiumCSS. It has its fair share of opinions and we’re not afraid to make these recommendations or even defaults, but in this case, it could potentially be a complication for implementers with an UX team willing to do some proper user research on footnotes/endnotes and what works best for these – as far as I can tell, there’s nothing available publicly for popup footnotes so we don’t even know if better options exist or not.

So I have to remember and mention this issue is not “either an internal link you go from and to, or what iBooks does.” There’s quite a lot of other design patterns you could test, and since user research is about iteration and lots of testing, I think it would be preferable this can be enabled/disabled in the app if we make it part of ReadiumCSS. As a disclaimer, as I have to occasionally do some user research myself, I also know results can be mind-blowing depending on the device, context, use cases, etc. so the largest spectrum ReadiumCSS can cover for such features, the better.

The implementation that would make sense for this should probably leverage ReadiumCSS flags. e.g. --RS__footnotes: readium-footnotes-hidden or something like that. And the selector would look something like night mode for instance.

On another note, there’s also endnotes, and ARIA roles that should probably be also taken into account as, in practice, usage is not as clearly cut as the best practices are.

Aside re administrative: @llemeurfr given the PR template is out of date, could we maybe explore having defaults for all the Readium repos? This is documented in “Creating a default community health file” and each project can override this default if needed. Please let me know if you want a separate issue in Architecture.

danielweck commented 4 years ago

In my opinion, this should not be a default in ReadiumCSS. In the readium-desktop "test app" users can choose to activate popup footnotes, or to use traditional hyperlinking. The "navigator" implements its own technical strategy to hide/show footnote contents located inside aside HTML elements (not based on namespace-aware CSS selectors, which have proved unreliable with the querySelector API even in modern Chromium, which Electron apps use)

tooolbox commented 4 years ago

let's forget the ICLA for now

Okay great.

ReadiumCSS could be used outside of our navigator implementation, so personally I would vote to have this feature disabled by default, and enabled by our navigator.

I think hiding by default is for once something that would be too much opinionated for ReadiumCSS.

In my opinion, this should not be a default in ReadiumCSS.

Okay, looks like we have a consensus here :) So now it's just a matter of whether this is part of ReadiumCSS at all, or if it's implemented in the Navigator (or App, more likely, based on discussion thus far).

In the readium-desktop "test app" users can choose to activate popup footnotes, or to use traditional hyperlinking. The "navigator" implements its own technical strategy to hide/show footnote contents located inside aside HTML elements (not based on namespace-aware CSS selectors, which have proved unreliable with the querySelector API even in modern Chromium, which Electron apps use)

  1. Sounds like we're moving away from the Navigator handling this, but I understand. Your point is that it's not decreed by ReadiumCSS.
  2. A little off-topic, but that seems not-very-good, to hide all <aside> elements. I'm curious why the JavaScript querySelector API even comes into play; isn't this purely a matter of CSS rules?

as far as I can tell, there’s nothing available publicly for popup footnotes so we don’t even know if better options exist or not.

It might not be important, but I'm not sure what you mean here. "available publicly"?

There’s quite a lot of other design patterns you could test

Again, might not be important, but I'm curious what your list of other design patterns are.

The implementation that would make sense for this should probably leverage ReadiumCSS flags. e.g. --RS__footnotes: readium-footnotes-hidden or something like that. And the selector would look something like night mode for instance.

Unfortunately I don't feel like I have enough of a grasp to know what to do with this. Like, I don't object to this, but I think I'm missing the big picture on what to do and how it would look or how to test it, or how this interfaces with the app and so on. I read through what you linked to and I get a vague concept but I couldn't write any styles.

For example, what is --RS__footnotes? I see other --RS things in the Night Mode stylesheets but they look like properties, not selectors? I also see --USER_xyz in the docs, I suppose RS is Reading System, and these are two levels/kinds of flags, or is there some kind of substitution going on and RS is considered a "user"? And I suppose readium-footnotes-hidden is one potential value; so readium-footnotes-shown would be the default? Who or what is responsible for setting defaults? I also see root.style.setProperty("name of var", ""); in the docs, I guess that's how the App controls the flags? I've never seen that sort of thing before, this a native browser API I guess?

These may all be dumb questions and I don't even expect a barrage of answers but I'm just illustrating how ill-prepared I am to submit a PR to implement this functionality. If you can spare the time to educate me, I will absolutely amend the PR.

On another note, there’s also endnotes, and ARIA roles that should probably be also taken into account as, in practice, usage is not as clearly cut as the best practices are.

I already touched on my stance for endnotes, but I don't care either way because my particular use case doesn't have them. I'm not sure what to do on ARIA roles, this is an area of accessibility that I'm not experienced in, so I am clueless on the relation and how this would impact whether something was hidden or not.

If there are a lot of different scenarios going on here between the types of notes and their roles and so on, it also might require a lot of different CSS variables or options, in which case perhaps it's better to leave it to the client/app/implementor to "vote with their stylesheet" so to speak.

I'm still down to amend the PR if it will help but currently it's looking like this doesn't belong in ReadiumCSS so I'm leaning towards closing.

JayPanoz commented 4 years ago

It might not be important, but I'm not sure what you mean here. "available publicly"?

Well let’s say I don’t doubt big companies have an UX team doing user testing, but what is certain is that they don’t make their research public – and, on a related note, even if they do, there is no guarantee there is no imbalance in favour of quantitative (A/B tests, data, etc.) versus qualitative testing (testing directly with users), cf. Google’s 41 shades of blue test pre-Material.

Such research not being made public has the unfortunate effect of turning “prior art” into a de facto standard to implement, with little room to improve through UX research as you have to start from scratch – a time-consuming an expensive process, although there exists DIY strategies. So publicly available research is mainly done in academia, but it’s fair to say it doesn’t necessarily do research on topics Reading Systems could leverage to improve the UX of ebooks.

what your list of other design patterns are.

@HadrienGardeur listed a few vanilla components used on Android yesterday in the Swift issue so I think it’s not really worth listing many more, an illustration of how complex it can be may indeed be preferable.

So let’s go outside ebooks for a moment, and check Tufte CSS, a popular stylesheet on the web.

The way they handle footnotes is context-dependant. When the screen is large enough, they are displayed as sidenotes in the margin, when it isn’t, then they become interactive and are displayed|hidden below the line where the link is.

I’m not saying this is good/bad, I don’t have any strong opinion here for lack of UX research, I’m just mentioning that implementers may even choose multiple patterns depending on the device/screen, the existing design system of the OS, the contextual layout, etc. And there may even be design patterns going out of date, new design patterns arising, etc. I mean, UX designers come with new ideas every single web, then they test those if they show some potential.

Unfortunately I don't feel like I have enough of a grasp to know what to do with this.

No worries, I can do that rather quickly in develop if we decide that should be part of ReadiumCSS. But some further details.

So when you see:

:root:--night-mode

Then :--night-mode is a custom selector in ReadiumCSS-config.

This is a draft spec but we can use it right now since we’re using PostCSS. That makes ReadiumCSS a little bit easier/powerful to author, and more customizable.

So you could add, in ReadiumCSS-config:

/* Pop-up footnotes */
@custom-selector :--popup-footnotes [style*="readium-popup-footnotes-on"];

Sorry changed the value so that it’s consistent with others.

And then your selector would become:

:root:--popup-footnotes aside[epub|type^="footnote"],
:root:--popup-footnotes aside[epub\:type^="footnote"] {
   display: none;
 }

Then the app has to add something like

--RS__popupFootnotes: readium-popup-footnotes-on

to html. The name of the CSS variable (--RS__popupFootnotes) doesn’t mean that much, as it’s typically not checked (but we need one), what’s important is the value.

But you’ll have to ask @mickael-menu for doing that in the app, as I’m not really familiar with the Swift codebase.

tooolbox commented 4 years ago

Thanks for the detailed explanation! I also had to skim this for a basic explanation on custom properties.

So here is where the defaults are set for night mode:

:root:--night-mode {
  --RS__backgroundColor: #000000;
  --RS__textColor: #FEFEFE;

  --RS__linkColor: #63caff;
  --RS__visitedColor: #0099E5;

  /* This can be customized but initial will re-use default value of the browser */
  --RS__selectionBackgroundColor: #b4d8fe;
  --RS__selectionTextColor: inherit;
}

And then used here:

:root:--night-mode a:link,
:root:--night-mode a:link * {
  color: var(--RS__linkColor) !important;
}

And I guess author stylesheets and/or Swift side can overwrite --RS_linkColor as it desires. My one remaining question is if there's a relationship between --RS__ and --USER__ properties, if there's some kind of cacade there or how that ties together.

Well let’s say I don’t doubt big companies have an UX team doing user testing, but what is certain is that they don’t make their research public – and, on a related note, even if they do, there is no guarantee there is no imbalance in favour of quantitative (A/B tests, data, etc.) versus qualitative testing (testing directly with users), cf. Google’s 41 shades of blue test pre-Material.

Such research not being made public has the unfortunate effect of turning “prior art” into a de facto standard to implement, with little room to improve through UX research as you have to start from scratch – a time-consuming an expensive process, although there exists DIY strategies. So publicly available research is mainly done in academia, but it’s fair to say it doesn’t necessarily do research on topics Reading Systems could leverage to improve the UX of ebooks.

I get it, you're saying there's no publicly available UX research on pop-up footnotes.

@HadrienGardeur listed a few vanilla components used on Android yesterday in the Swift issue so I think it’s not really worth listing many more, an illustration of how complex it can be may indeed be preferable.

That's not really what I was aiming at; those are all variations on a pop-up and not really different UX. To this point I have seen two pieces of UX:

I hope that makes sense?

So let’s go outside ebooks for a moment, and check Tufte CSS, a popular stylesheet on the web.

The way they handle footnotes is context-dependant. When the screen is large enough, they are displayed as sidenotes in the margin, when it isn’t, then they become interactive and are displayed|hidden below the line where the link is.

I’m not saying this is good/bad, I don’t have any strong opinion here for lack of UX research, I’m just mentioning that implementers may even choose multiple patterns depending on the device/screen, the existing design system of the OS, the contextual layout, etc. And there may even be design patterns going out of date, new design patterns arising, etc. I mean, UX designers come with new ideas every single web, then they test those if they show some potential.

Okay, now that is a little more of what I'm talking about, and that's fascinating.

So then, returning to the original statement(s):

I think hiding by default is for once something that would be too much opinionated for ReadiumCSS. ...

There’s quite a lot of other design patterns you could test, and since user research is about iteration and lots of testing, I think it would be preferable this can be enabled/disabled in the app if we make it part of ReadiumCSS.

What I get out of all this is that if someone wanted to use R2/ReadiumCSS for something like Tufte CSS, it's going to be annoying if the <aside epub:type="footnote"> elements are hidden by default; rather, they would want to show them above a certain screen width threshold and hide them below that, in addition to whatever else they were doing. Okay, I get that use case and see how, even if you wanted to put a CSS rule in for this, you would want it off by default.

(...Come to think of it, one wonders if such a Tufte implementation would even want to use ReadiumCSS if they're doing a whole fancy margin thing, but I guess it's a good starting framework.)

Okay great, I feel pretty settled. Based on discussion here and in https://github.com/readium/swift-toolkit/issues/139 it seems like we don't want pop-up footnotes as a core Readium feature, but rather implemented in the App, so I don't think there's a lot of value to this change. Okay to close?

JayPanoz commented 4 years ago

And I guess author stylesheets and/or Swift side can overwrite --RS_linkColor as it desires. My one remaining question is if there's a relationship between --RS and --USER properties, if there's some kind of cascade there or how that ties together.

Indeed.

So the relationship between --RS__ and --USER__ is an attempt at making CSS Cascading and Inheritance a little bit clearer. Given we don’t have any “standard” syntax or API to handle that from ReadiumCSS yet, --RS__ is used for user agent declarations, while --USER__ is used for users’ declaration (through user settings).

In practice, they’re all exposed to the app, so you can change/override them dynamically. Only does --USER__ makes it clearer it is used for something the user can change/override.

rather, they would want to show them above a certain screen width threshold and hide them below that, in addition to whatever else they were doing. Okay, I get that use case and see how, even if you wanted to put a CSS rule in for this, you would want it off by default.

So as you mention, that is the crux of the issue, as you could copy Tufte CSS’ design pattern w/o even using it, or come up with another one that doesn’t exist yet, and you obviously wouldn’t want ReadiumCSS in your way – you’d want to be in control.

Okay to close?

Maybe we can vote, as there still could be some value in providing a flag the app can enable/disable, and we’re now doing that for such issues or questions.

So if people would like to have a ReadiumCSS flag they can enable/disable to hide (aside) footnotes, use 👍.

Otherwise, use 👎

tooolbox commented 4 years ago

I'm more or less in favor, I think the remaining question is which kinds of elements are targeted and whether they have separate flags:

Ref: http://kb.daisy.org/publishing/docs/html/epub-type.html#ex

mickael-menu commented 4 years ago

I'll abstain for now. I'm in favor of adding this feature, since ReadiumCSS is specialized for EPUB, but only if there's a clear list of elements to hide that is more or less "standard".

If this list might change depending on the app, then I think this should be handled by CSS injection in the Navigator.

HadrienGardeur commented 4 years ago

That's not really what I was aiming at; those are all variations on a pop-up and not really different UX.

That's actually incorrect.

The side sheet on Android/Material Design can be docked to the right of the window when you reach a certain size.

Displaying notes on the side is IMO a very common pattern both in print and in digital publications and should be included in that list:

  • Display the footnotes normally in the text, hyperlinks jump to them in the main body of text.
  • Hyperlinks to footnotes display the content in and the footnotes may or may not be hidden in the main body of the material.

Where the footnotes are displayed would of course be up to the reading app entirely. Displaying them on the right is natural for ltr text, but I can imagine that this could make more sense on the left for rtl text or at the bottom for vertical text.

Such a note panel would not necessarily behave like a pop-up, it could for example display all notes for a given chapter (or more specifically, for all notes in a given resource of the reading order) rather than a single one after clicking on it.

JayPanoz commented 4 years ago

I'll abstain for now.

Yeah same, I don’t have any final answer yet.

If this list might change depending on the app, then I think this should be handled by CSS injection in the Navigator.

That’s one of the complexities indeed, as some may want to handle endnotes as well, or also target other elements e.g. section, div, and so on and so forth.

The fact Daniel’s navigator implements its own technical strategy is kind of another hint, as I know he would not have hesitated to open an issue if it were obviously better to handle it through ReadiumCSS at the time, so I’m currently split.

At some point in the future, it wouldn’t surprise me that if it became a “convenience declaration/flag” in my thinking i.e. “here is a ready-to-use flag to replicate iBooks’ pattern. It’s disabled by default. If you want to use another pattern, then use CSS injection in the navigator to have more control over it.” I can’t even categorise it “compatibility”, as it’s UX and not purely technical.

Not sure this would be a good middle ground but at least it would make it clear what to do.

tooolbox commented 4 years ago

If this list might change depending on the app, then I think this should be handled by CSS injection in the Navigator.

Out of curiosity, is there a good way to do CSS injection from the app level? I don't see anything really set up for that.

Such a note panel would not necessarily behave like a pop-up, it could for example display all notes for a given chapter (or more specifically, for all notes in a given resource of the reading order) rather than a single one after clicking on it.

Okay, so you get a side panel displaying all notes for a chapter. It's either visible all the time, or based on screen width, or after the user taps one of the hyperlinks.

I guess, in the end, you're hiding the notes in the main body of the text, and the noteref hyperlinks aren't jumping to them—they're potentially activating a way of displaying them, possibly more than one note as well—so I would categorize it with the 2nd option in my list. I do see how it's different from a pop-up though.

mickael-menu commented 4 years ago

Out of curiosity, is there a good way to do CSS injection from the app level? I don't see anything really set up for that.

Not yet, but that's definitely on the roadmap.

danielweck commented 1 month ago

In my opinion, this should not be a default in ReadiumCSS. In the readium-desktop "test app" users can choose to activate popup footnotes, or to use traditional hyperlinking....

https://github.com/readium/readium-css/pull/83#issuecomment-617861214

4 years on, I still feel the same about the current "sensible" default in ReadiumCSS (which is PR proposal changes), based on our experience shipping the "EPUB popup footnotes" feature in Thorium. There are users who like the modal popup footnotes UX because the reading context / displayed document area doesn't change, and hitting the escape key (or clicking outside the modal) immediately brings users back to the note call site (hyperlink), in a screen reader / accessible manner. In this case we hide targeted aside element that are marked as "note contents". There are also user who do not like the popup modal UX, and prefer linking into note contents and relying on the "back link" to return to the hyperlink callsite. In this case we do not hide aside notes, of course.

JayPanoz commented 3 weeks ago

I guess it’s safe to say this won’t be merged as it’s app and UX-dependent. But with ReadiumCSS’ update in philosophy of becoming an unopinionated library providing styles that can be applied reliably, we could also discuss whether having utilities for such features could help developers.

For instance, it could provide a flag to hide popup footnotes but not make it a default. And if the app is implementing popup footnotes, it could use this flag. Otherwise, it doesn’t make any difference.