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

Screen rotate support for new ui #732

Merged
merged 14 commits into from
Feb 5, 2013
Merged

Screen rotate support for new ui #732

merged 14 commits into from
Feb 5, 2013

Conversation

houqp
Copy link
Member

@houqp houqp commented Feb 2, 2013

It's quite a big patch, I partially rewrote the screen and touch input module. Basically the internal API changed a lot.

The new implementation is based on the idea from @dracodoc, as discussed in #573. The screen rotation feature is device independent now, i.e. it don't need to interact with kindle framework any more. That's means it will work for other devices like Kobo and there will be no more glitches when switching into/out of screen saver :)

However, it's actually not finished yet. There are still a couple of small bugs that need to be fixed. I would like to send out the PR first so if some one else plan to work on the input or output module will be notified of the changes.

@@ -26,7 +26,9 @@ KoptOptions = {
name="screen_mode",
name_text = "Screen Mode",
item_text = {"portrait", "landscape"},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge conflicted at this line. In the new_ui_code branch it ought to be toggle = {"portrait", "landscape"},
CONFLICT (content): Merge conflict in frontend/document/koptinterface.lua

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's the problem. The default arg for this toggle should be decided on run time. So far the only solution I can think of is to set this up by changing self.options in ReaderConfig's onSetDimensions handler.

Is it the right way to do so?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good. We should also see if it will work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think at some point we need to move the config options in koptdocument and credocument to ui components.

For example, for this screen rotate feature, it's entirely not related to document data, so it dosen't make much sense to put it in *document.lua, since *document.lua should only provide document manipulation APIs.

Also some of these options has attributes like width , height, font size, font, font text, etc, which are also document independent.

config dialog UI is very handy so we might need to use it in somewhere else not just for reader, for example: help page, file chooser (Other devices like Kobo need this), etc. These are all document independent features so where should we put the option definitions then?

Yes, your previous point is correct, all these attributes are like css, thus should not put into UI implementation code. Then how about making something like a ui-data directory and put related options definition there?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving config options to a separate file is a good idea. We could think about the clean up work later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it doesn't work. I even try to set the default_arg in readerui.lua, before calling ReaderConfig:new, I still get following error:

# toggle position: 2
lua config error: ./frontend/ui/config.lua:238: attempt to perform arithmetic on field 'position' (a nil value)

More hints on how to use toggle?

@chrox
Copy link
Member

chrox commented Feb 3, 2013

It works great on emulator. I will then test it on paperwhite.

@houqp
Copy link
Member Author

houqp commented Feb 3, 2013

Wait, it's broken right now. The toggle is blank and when I click on it I get following error:

# toggle position: 2
lua config error: ./frontend/ui/config.lua:238: attempt to perform arithmetic on field 'position' (a nil value)

I still cannot find a way to set default_args on run time...

@chrox
Copy link
Member

chrox commented Feb 3, 2013

Probably we can set default_args Screen:getWidth() > Screen:getHeight() and "landscape" or "portrait" at runtime.

@houqp
Copy link
Member Author

houqp commented Feb 3, 2013

@chrox , good idea, this is much cleaner 👍

Don't merge it yet as I am working on some other fixes.

chrox added a commit that referenced this pull request Feb 5, 2013
Screen rotate support for new ui
@chrox chrox merged commit 756785e into koreader:new_ui_code Feb 5, 2013
@houqp houqp deleted the rotate branch February 5, 2013 15:29
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.

2 participants