Open GoogleCodeExporter opened 9 years ago
Hi Vldmrrr, thanks for patch.
In future it would be best to submit it directly through pull request on github
-
https://github.com/max-kammerer/orion-viewer
Original comment by mikhael.bogdanov
on 14 Mar 2013 at 3:03
I thought you might want to review the changes first before applying it, as it
touches things outside edge specific code. But maybe pull request allows to do
just that - I am not very much familiar with git. Anyway, will use that next
time.
Original comment by vldm...@gmail.com
on 14 Mar 2013 at 3:09
Yes, github pull requests allows to make code review before merge.
Original comment by mikhael.bogdanov
on 14 Mar 2013 at 4:08
I have made some refactorings and merged keyboard support to head, service part
now avaliable in "edge_patch" branch (it's avaliable both on google code and
github)
Original comment by mikhael.bogdanov
on 16 Mar 2013 at 8:15
[deleted comment]
I believe that i don't introduce any bug)
Original comment by mikhael.bogdanov
on 16 Mar 2013 at 8:28
I tested both branch and master. The branch work as good (or as bad) as the
patch I submitted.
What surprised me was that master branch actually achieves the intent of the
branch as well without using service. That is, the viewer can be put into
background, and pages can still be turned with reader buttons. So looks like
this whole service messing is unneeded.
The only somewhat problematic behaviour is that returning reader into
foreground on tablet opens files selector on the tablet. Problematic because
the intent of user might be just change preferences while keeping the same book
open. But that is the minor problem, as selecting the same book does preserve
the current reading position, so it is somewhat acceptable, just one extra step.
Thanks
Original comment by vldm...@gmail.com
on 16 Mar 2013 at 3:45
You could try to set "Open resent book on start" in application settings maybe
it help you
Original comment by mikhael.bogdanov
on 17 Mar 2013 at 6:06
Selecting "Open resent book on start" does not resolve the problem of bringing
the reader back into foreground on tablet screen: with this setting the reader
jumps back to the page that was visible at the time when it was hidden on
tablet screen. So this might be more confusing than default behaviour.
There is more serious problem - after returning the reader to foreground and
pressing HW button the application crashes with NullPointerException rooted in
EdgeKbdThread in nextPage(). It turns out that after coming back to foreground
the new device with while the old one is still hanging around.
I created a fix stopping the thread after switching to foreground. I was going
to submit it through pull request on github as you suggested, even signed up
for account there just for that, but looks like it is quite a convoluted
process - it appears i need first to create a clone repo on github and then
dance around that. Seems to much of a trouble for for a simple patch. So I
would again add it as attachment here for now.
Original comment by vldm...@gmail.com
on 17 Mar 2013 at 5:26
Attachments:
Hi, after forking orion repo you could add it as additional remote repository
for your local orion sources and comit to it. See
http://git-scm.com/book/en/Git-Basics-Working-with-Remotes. It's best to create
separate branch for each pull request patch otherwise you will get problems
with merge from orion master repo.
For small patches you could also directly edit specified file from browser,
after saving it github automatically create pull request)
Original comment by mikhael.bogdanov
on 18 Mar 2013 at 4:09
If there is a need, I could test of course, unfortunately it is probably all
that I am able to do ...
Original comment by gbarn...@gmail.com
on 18 Mar 2013 at 4:19
Merged with small changes
Original comment by mikhael.bogdanov
on 23 Mar 2013 at 5:46
Original issue reported on code.google.com by
vldm...@gmail.com
on 13 Mar 2013 at 5:01Attachments: