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

Margins are no longer settable #179

Closed
ryanackley opened this issue Aug 26, 2014 · 8 comments
Closed

Margins are no longer settable #179

ryanackley opened this issue Aug 26, 2014 · 8 comments

Comments

@ryanackley
Copy link
Contributor

Recent changes related to margins seems to remove the ability to change the margin size. Not sure if this is by-design. @danielweck is on holiday and he made those changes.

We need to determine if we will do auto-margins moving forward. If so, we need to remove the setting from the dialog. If not, we need to make them settable again.

@danielweck
Copy link
Member

Copy from the mailing-list:
https://groups.google.com/forum/#!msg/readium-dev/_ja4s8TPazI/36JCIvvA4WgJ

I have just double-checked on my side (pulled fresh code from the
develop branches), and it seems to work fine (Safari OSX, cloud
reader, "accessible EPUB3" built-in ebook, default font size).
I have a primary 1280 x 800 display, and I resized the browser window
to verify the reflowable page width algorithm (my secondary display is
much higher resolution, but that works too).
I can see a uniform "whitespace" border around each page (which
doubles in between two adjacent pages of a synthetic spread), and the
previous/next page navigation arrows are rendered within the
left/right margins. The "column" width varies between a minimum and a
maximum threshold, to avoid long runs of text, and narrow lines.

As an alternative point of reference, iBooks renders reflowable EPUBs
in the same way.

@ryanackley
Copy link
Contributor Author

Neither Ric or myself could change the margins with the slider in the settings dialog. Therefore, the question is are the margins settable from the settings dialog?

If not, I don't necessarily see this as bad or good but I think it should have been made more clear to Bill and Ric. It's a little bit of a radical departure from what we are currently doing. Based on our previous discussion, I assumed that the user would still be able to choose from a range of margins. Also, if we decide to go this route we should remove the margin slider from the settings dialog.

@danielweck
Copy link
Member

In other words: the "margins" setting controls the fixed size of the border around each individual page. I feel that this setting would be more useful if larger values were allowed (right now the difference is hard to tell). Note that eventually, we should probably group this setting with other readiblity enhancements such as:
#114

The optimal width of a line of text within a given page is a separate problem altogether. The getWidthFactor() approach had severals flaws (refer to the links below), one of which was the inadequate shrinkage of text content. Right now, Readium achieves a layout similar to iBooks, based on a commonly-accepted threshold in print and web design (minimum and maximum number of characters, with assumption on font size). iBooks does go much higher in font size and consequently seems to have a separate threshold to avoid tiny line lengths (auto switch to single spread, instead of maintaining a double page spread in landscape mode).

#151
readium/readium-js#68
readium/readium-shared-js#93

@danielweck
Copy link
Member

Attached "accessible EPUB3" screenshots, when using the minimum and maximum margin values from the settings dialog. The navigation toolbar and page buttons are hidden.
Note that the previous/next page buttons are embedded within the left/right margin whitespace (to avoid obscuring content) so sufficient space is always allocated for them (which explains why the first screenshot shows a thicker left/right margin than expected: the previous/next arrow width is greater than the minimum content margin).
When resizing the browser window (try with iBooks as well, on the same screen with the same book), you can observe the text width min/max thresholds. On a very large display, note how the text does not stretch to silly widths (but remains within the optimal range). Equally, a synthetic spread switches to a single page when the text width reaches the mimimum boundary (assuming the spread was not forced by user or author, giving the Reading System freedom to use an optimal layout).

PS: demo of a similar min/max width threshold pagination algorithm, switching to 3 columns on large displays:
http://sorotokin.com/adaptive-layout/driver.xhtml#x=tutorials/01-basic-page/index.xhtml

minmargin

maxmargin

@danielweck
Copy link
Member

Follow-up to ReadiumJS conference call:
I would suggest shipping the margin setting as it currently is. If people start complaining about the change of behaviour introduced in v1, then we'll know that we need to give users control over the text width size: min/max thresholds, which will consequently allow more whitespace on either left/right sides of the browser window, thereby emulating the previous "large whitespace / narrow columns" behaviour which some users may have been used to (... although I would speculate that not many people actually used the margin settings to alter/improve their reading experience, but that's just my personal un-scientific impression).

We have v1+ issues for adding new "readability enhancements" features, such as line height, word spacing, etc. which can be eventually grouped with the margin setting. We also know that the settings dialog needs to be re-organised, features removed / hidden, etc. (e.g. keyboard shortcuts customiser, page transitions).

@danielweck danielweck added this to the v1+ milestone Sep 3, 2014
@danielweck
Copy link
Member

Follow-up: there is one user feedback on the IDPF forum about the "margins" slider setting not presenting the same behaviour as before (as discussed above). Just as @rkwright asked, we need more information about screen size, browser window dimensions, font size, etc. (it may be that the user is forcing single page mode, expecting the text to stretch to most of the width of the screen)

At any rate, if users wish to see larger runs of text across the screen (just like regular web pages that do not use optimal / conventional min-max width thresholds), then we may need to consider adding a new setting, or revamp the "margins" slider setting which currently affects only the fixed-size whitespace area around each page of text.

http://idpf.org/forum/topic-1773

@danielweck
Copy link
Member

Here, we need to do two things:

  1. make the Chrome extension behave like it used to: the "margins" slider in the settings dialog should visibly affect the left/right blank areas on either sides of the single or double page layout, in addition to setting the constant margin / blank border that surrounds each page (which in turn determines the width of the dividing area between two consecutive pages within a synthetic spread).

  2. make it possible to configure the width thresholds of text "columns":

  • min value so that a two-page spread of reflowable documents automatically switches to single-column mode when the lines of text become unreasonably narrow,
  • max value so that text does not run across excessive horizontal distances.

Right now these values are hard-coded, see:
https://github.com/readium/readium-shared-js/blob/develop/js/views/reflowable_view.js#L510

Some users prefer wide columns (even as far as full stretch on landscape displays), whilst others need to be able to constrain the horizontal runs of text to a reasonable distance (for accessibility reasons). Because of the broad variety of personal preferences / needs, it would be nice to expose the min-max customisation in the settings dialog of the Chrome extension. This could replace or supplement the existing "margins" slider referred-to in (1).

@danielweck
Copy link
Member

Addressed via #498
(fine-tuning of slider values in develop branch)

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

3 participants