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

Output html page width #25

Closed
fohrloop opened this issue Apr 3, 2021 · 8 comments
Closed

Output html page width #25

fohrloop opened this issue Apr 3, 2021 · 8 comments
Labels
enhancement New feature or request ToDo solution known, but not implemented yet

Comments

@fohrloop
Copy link

fohrloop commented Apr 3, 2021

Just an idea: Would it be nice to have an option for having some maximum page width for the output html? I mean, now it is possible through by adding extra css:

// extra.html
<style>
	div.gist-file {
		max-width: 800px;
		margin: 0 auto;
	}
</style>

and

gh_md_to_html.main(
    inputfile,
    extra_css="extra.html",
)

I guess this is something people would like to have very commonly. As this package aims to remove the hassle (with alternatives like pandoc), there could be a reasonable default page width. The default (max) page width could be or example 800px and it could be always centered. I'm thinking something like this

gh_md_to_html.main(
    inputfile,
    extra_css="extra.html",
    page_width=900,
)

to control the page width and page_width=None would remove max width altogether.

@phseiff
Copy link
Owner

phseiff commented Apr 3, 2021

Thanks for the suggestion! I will look into this later this evening, but it sounds like a very good idea so far.

@phseiff phseiff added the enhancement New feature or request label Apr 3, 2021
@phseiff
Copy link
Owner

phseiff commented Apr 3, 2021

I think that this would go very well with adding an option to automate this little customization suggested in the README. I am questioning whether it would be possible to add an option that handles both at once - I think an option for the thing mentioned in the README would also passively be an option for the thing you suggested, but somewhat better since modifying the margin on the left and right side of the text makes the resulting html more responsive to size changes than madifying the width in which the text is displayed?

It will take some days until I find time to implement this, though, since time is currently somewhat sparse for me.

@fohrloop
Copy link
Author

fohrloop commented Apr 4, 2021

Yeah it could be probably a good way to implement the feature in such way that

  • The page width has reasonable (good looking) default, and it could be changed from python as argument to main(). The page structure related arguments could also be grouped into one input argument dictionary, like styling_kwargs, if it feels like there are going to be a lot of those. Anyway, I think that the page width is one of the only things people need to customize fast, and it's nice if users do not have to know CSS or front-end development to do that. And under the hood the page_width (python arg) would convert to max-width (CSS) to make the page responsive.
  • The page margins could be growing automatically by default, as it is most probably the most common use case. ( 0 auto). This would make the page always centered.
  • All the styling should still be changeable by using custom CSS, at least with !important. (this would then rule out the option that inside prototype.html there would be automatic inline css for the components?). This would give free hands to anyone who wants to do a bit more customization.

What kind of CSS you think would be good? I like the idea of modifying the prototype.html automatically.

@phseiff
Copy link
Owner

phseiff commented Apr 4, 2021

Just to be sure we are both talking about the same thing, what kind of centering are we talking about here? Is it about centering the box in which the text is displayed (the thing between the red lines in the following image):

image

or is it about centering the text within that box:

image

Because I think the former one would probably work best by specifying a width (and centering the whole box on the page), whilst the second one (which is the thing I mentioned in the README) would work best by specifying a margin to be added to the left and rigth side of the page.

I originally thought we where both talking about the second one, but it appears to me that you're actually referring to the former one (which is good, since it means we don't actually have a disagreement on whether to use max-width or margin for the centering).


About your suggestions:

The page width has reasonable (good looking) default

I don't really think that this is a good way to go, since it would change the way the pages are displayed by default. Many people (at least around 10, by my estimates) use this tool already, some of them in automated tools and pipelines, like me (who uses it in a website builder), and gh-md-to-moodle, which uses gh-md-to-html in the background. Changing the way the generated html looks by default, even if it makes the html better-looking, would silently break some of these tools - the project would therefore break backwards compatibility. This is not something a reliable tool should do, so I'm opposed to the idea to change the default.
Changing the default would also change it from GitHub's default, which is another reason why I'm opposed to doing it.

The page structure related arguments could also be grouped into one input argument dictionary, like styling_kwargs, if it feels like there are going to be a lot of those.

I'm not sure if that would work out well, since grouping arguments together in the Python API would introduce differences between the Python API and the command line interface, which would be counter-intuitive and require additional documentation to read (for the user) and to maintain (for me).

All the styling should still be changeable by using custom CSS, at least with !important. (this would then rule out the option that inside prototype.html there would be automatic inline css for the components?). This would give free hands to anyone who wants to do a bit more customization.

As long as the inline-CSS added by page_width comes before the css added by using style_pdf, the style_pdf-option would overwrite page_width anyways, as far as I'm aware, so using !important shouldn't even be necessary? I'm not an expert when it comes to CSS, so please feel free to correct me on this if I am wrong.

I like the idea of modifying the prototype.html automatically.

Me too; I try to keep as much as I can within the generated HTML rather than moving it to extra files.


But overall, apart from the two small details I outlined above, I agree with you that adding an option for page_width (without changing the default value, though) would be a good idea. 👍
I will implement this once I find time to work some more on it (and maybe box_width would be a better name than page_width, since it makes it clearer what it refers to?).

@fohrloop
Copy link
Author

fohrloop commented Apr 4, 2021

I'm not sure if that would work out well, since grouping arguments together in the Python API would introduce differences between the Python API and the command line interface, which would be counter-intuitive and require additional documentation to read (for the user) and to maintain (for me).

Oh yeah good point! Then putting the page_width as direct argument to main() is probably the way-to-go.

As long as the inline-CSS added by page_width comes before the css added by using style_pdf, the style_pdf-option would overwrite page_width anyways,

This would be my first guess, too. So just inserting the additional, auto-generated css to the top of the html would satisfy the requirements well.

what kind of centering are we talking about here?

I think I'm talking about the first one. To be explicit, created the html from the README file, first without any custom CSS:

gh-md-demo1

and then with the proposed changes:

gh-md-demo2

These are both recorded with a maximized browser window. The second one looks more natural and is easier to read, as optimal line length for reading is 45-80 characters. Since GitHub itself embeds the bordered region, the line length is naturally smaller:

image

It probably does not matter a lot if the space is inside or outside the box/border, but imo the border looks kinda nice around the text :)

@phseiff
Copy link
Owner

phseiff commented Apr 4, 2021

optimal line length for reading is 45-80 characters

Oh wow, that's pretty interesting; I never knew there was so much to it!

[GIFs]

These GIFs put things a little more into perspective for me; I see how you meant it, now.

It probably does not matter a lot if the space is inside or outside the box/border, but imo the border looks kinda nice around the text :)

Full agreement :)


I will implement this, then (unless you want to make a pull request for that yourself, of course) 👍

@phseiff phseiff added enhancement - accepted ToDo solution known, but not implemented yet and removed enhancement - accepted labels Apr 4, 2021
@phseiff
Copy link
Owner

phseiff commented Apr 5, 2021

Fun fact: Someone appears to have written an entire wrapper around gh-md-to-html, just for the purpose of adding some boxing like this (and to reduce the command line interface to something like gh-md-to-moodle in.md out.html, which is then internally translated to gh-md-to-html in.md -w . -n out.html), plus other things I do not fully understand yet.

So yeah, customizing gist-file's max-width seems to be a much more common use case than I originally anticipated.

Edit: I learned there is a lot more to said wrapper; it's very cool, actually.

@phseiff phseiff closed this as completed in 4a6a254 Apr 9, 2021
@phseiff phseiff reopened this Apr 9, 2021
@phseiff phseiff closed this as completed in 522c66f Apr 9, 2021
@phseiff
Copy link
Owner

phseiff commented Apr 9, 2021

@np-8 This is implemented now; you can update to v1.8.0 and use --box-width 25cm to change the width of the page now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ToDo solution known, but not implemented yet
Projects
None yet
Development

No branches or pull requests

2 participants