Skip to content
This repository was archived by the owner on Sep 22, 2020. It is now read-only.

add css file selection menu for crengine #721

Merged
merged 11 commits into from
Jan 22, 2013

Conversation

houqp
Copy link
Member

@houqp houqp commented Jan 18, 2013

No description provided.

@ghost
Copy link

ghost commented Jan 18, 2013

Do you see some reason for user to switch stylesheet on the fly?

@tigran123
Copy link
Member

@Nupogodi Welcome back. The only reason mentioned so far was the ability to toggle hyphenation on and off. Do you think it would be better to implement this (hyphenation control) as a separate toggle instead? Or maybe you have an altogether different better idea?

@chrox
Copy link
Member

chrox commented Jan 18, 2013

It seems that font-family property in stylesheet is overwritten by selected font. And the font-family definition is never used in the following CSS snippet.

p {
    border: 0;
    display: block;
    font-family: FZYaSong-M-GBK, 宋体, serif;
    font-size: 1.0em;
    line-height: 1.5;
    margin: 0;
    padding: 0;
    }

And I think it's better to use CSS shipped with the EPUB file by default and leave the freedom of choosing pre-defined CSS to users.

@ghost
Copy link

ghost commented Jan 18, 2013

I'm not exactly back (not enough time and inspiration to learn new KPV features), just a curiosity about the reason to switch css's. Hyphenation? Some time ago I've already introduced some corrections allowing me to select between "no hyphenation", "algorithmic hyphenation" (universal method -- it works properly with Russian texts, but gives terrible results when applied to, say, German books) and tex-dictionaries available in crengine distribution -- iirc, in crengine/data/hyph. I'm not sure whether this way is right or wrong, but it works somehow.

@houqp
Copy link
Member Author

houqp commented Jan 18, 2013

@chrox, font-family overwritten by selected font is somehow expected. I guess the setting in css should has lowest priority.

Using the embed CSS is indeed the right thing to do, I totally missed that ;p Will check this on weekend.

@Nupogodi , long time no see ;p

Currently for me the most useful thing of switching css is to adjust indent. One of my book is rendered with very large indent, which looks very uncomfortable.

@ghost
Copy link

ghost commented Jan 18, 2013

@houqp , iirc, crengine generally allows to switch on/off the embedded styles -- did you try to toggle them via self.doc:setEmbeddedStyles(...) ?

static int setEmbeddedStyles(lua_State L) {
CreDocument *doc = (CreDocument
) luaL_checkudata(L, 1, "credocument");
doc->text_view->doCommand(DCMD_SET_INTERNAL_STYLES, luaL_checkint(L, 2));
return 0;
}

@houqp
Copy link
Member Author

houqp commented Jan 18, 2013

@Nupogodi , aha! That's what I am looking for :) Thanks a lot, will test it tonight.

@dracodoc
Copy link

@Nupogodi , this feature was suggested by me because KPV was using its own css for all epubs, so that single css will not have best result for these different configurations:

  1. Chinese Epub and English epub will have different fonts. KPV can have a default font for crengine but that means user need to manually select font again and again for the other language epub.
  2. Some epub have text indented, some have no indent and expect the css to define indent, so you will need 2 different configuration for these.
  3. And of course there are many other css combinations to best suit different epubs.

If KPV was using the epub embeded css, this feature may not seemed useful at first. But I think not using embeded css actually is good, because many epubs are not build with best format, or there could always be some settings you don't like, so have some preset css is a easier method.

@tigran123 , I think we discussed this before,
https://github.com/hwhw/kindlepdfviewer/issues/710
I talked about all kinds of possible usage for custom css, and you acknowledged them. I don't know why would you say the only mentioned use for custom css is for hyphenation toggle?

I also cannot agree about your comment "The real strength and value of KPV is in the PDF and DjVu area, not in crereader." https://github.com/hwhw/kindlepdfviewer/issues/711
At least for me, the value about DjVu is ZERO, and epub feature can take 30-40% in my usage.

@houqp
Copy link
Member Author

houqp commented Jan 19, 2013

Ha, yes, forgot to mention that the idea of customized css is from @dracodoc :)

@Nupogodi , did you succeed with doc->text_view->doCommand(DCMD_SET_INTERNAL_STYLES, 1) before? Seems that it's not working for me :(

@ghost
Copy link

ghost commented Jan 19, 2013

@houqp, i did not test it, but i suggest that "1" (default value) causes crengine to use the styles embedded into the book, whilst "0" means the external styles (css)
@dracodoc, your idea might work, i'm just a bit afraid of the css-file number, that drastically increases with the number of parameters to be adjusted. i'd suggest to add only two toggles -- for embedded fonts and for embedded styles -- and activate them whenever i see bad formatted book. Saving these toggles to the book history also looks quite reasonable. It's nothing more then just my personal feeling... i just did it this way in my outdated kpv-version, but unfortunately never have an opportunity to test how it works -- i mostly read fb2's without embedded styles & fonts

houqp added 3 commits January 22, 2013 06:26
* now css argument is removed from newDocView call.
* when setStyleSheet method cannot read given css file, it
  clears all the applied style.
* add setEmbeddedStyleSheet method.
* now we only use Embedded CSS by default
* add toggleEmbeddedStyleSheet methods
@houqp
Copy link
Member Author

houqp commented Jan 21, 2013

OK, the DCMD_SET_INTERNAL_STYLES is actually working :) I was misled by it because when I set it to 1, crengine won't clear all the existing styles, but just apply the embedded styles to current document.

Thanks @Nupogodi

Now we only use embedded css by default, and use can change to other css by using the menu and toggle embedded css on/off in the reader config dialog.

chrox added a commit that referenced this pull request Jan 22, 2013
add css file selection menu for crengine
@chrox chrox merged commit 0bb3ede into koreader:new_ui_code Jan 22, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants