Linespace customise FR

[hayden-MB]

Pull request for FR #1477. Tested on bookdown-demo.

Rationalle for options: 1.0, 1.5, 2.0, 2.5 and 3.0 are standard options while 1.7 was the default.

Happy to discuss any of this.

[cderv]

superseeding #1478 - just as note for myself

[yihui]

Two questions:

1. What did you change in inst/resources/gitbook/css/style.css? I feel it shouldn't be necessary to change this file (I think all relevant CSS can be added to plugin-fontsettings.css).
2. Instead of hard-coding a few line height choices, would it be better to only add two buttons (one for increasing, and one for reducing line height, say, by 0.1)? By clicking these buttons, you change the data-line-height attribute of the element .book .book-body .page-inner section, and in CSS, you set line-height: attr(data-line-height). Then readers will have full freedom of choosing the most comfortable line spacing.

[hayden-MB]

1. I removed a line which said 'line-space: 1.7' because it was overwriting the linespace menu. I could maybe do !important in plugin-fontsettings.css but that feels sligtly clunky. Will do whatever you feel is best though!
2. I can work on this.

[hayden-MB]

That should be the toggle option. Let me know what we should do on the CSS issue.

[yihui]

1. plugin-fontsettings.css appears later than style.css, which means for the same CSS selector, plugin-fontsettings.css has precedence over stye.css, so I don't think you need !important (which we should typically avoid using).

2. I didn't mean enumerating line-height values in CSS, but using the CSS function attr(), which gives you full freedom to use any line height specified in an attribute of the DOM element. I hope that's clearer now.

Thanks!

[hayden-MB]

Hi there

1. I would have thought this too, but it only works if I remove this line from the style.css. Not sure how to deal with this.

2. Ah, thank you. I simply mirrored the style from font size which is a list of options, presumably to restrict options to a reasonable range.

I have committed your changes to my fork and tested them and they simply don't work. The spacing does not change with the buttons.

[hayden-MB]

I don't think attr() works for line-height. This should work, I also added a minimum line height of 1 to prevent users from crossing text.

[hayden-MB]

There is an issue that if you click reduce a bunch of times after you have reached minimum, you have to click increase the same number of times to start making a difference. Not sure how to fix this without hardcoding options. This may be why font size was done in this way.

[yihui]

You are right. Sorry I forgot that attr() currently only works for content: https://developer.mozilla.org/en-US/docs/Web/CSS/attr

There is an issue that if you click reduce a bunch of times after you have reached minimum, you have to click increase the same number of times to start making a difference. Not sure how to fix this without hardcoding options.

When fontState.spacing <= 10, we stop decreasing it.

BTW, is it still necessary to remove the default line-height from style.css? The style attribute usually has the highest precedence and will override most rules defined in external stylesheets.

Please also add a bullet item to NEWS.md.

[hayden-MB]

Yes we can now return the css to how it was as there is no changes to even the fontplugin css anymore. Should be that all the changes are in the JS page.

[hayden-MB]

Not sure why accepting that change seems to have caused tests to fail? Any ideas?

[yihui]

It seems that style.css was not restored.

Not sure why accepting that change seems to have caused tests to fail? Any ideas?

I think you can ignore that. Looks like a change in the dev version of Pandoc irrelevant to this PR. We'll look into it.

Thanks!

[yihui]

I've tweaked the rest of minor things and the PR is ready to merge now. Thanks again!