Introduce Export/Import (to file) Reading List #1163
Conversation
|
@veloman-yunkan The readBookmarkFile simply appends all bookmarks to the library. This can create duplicate bookmark entries. Should we do the elimination on Kiwix-Desktop or libkiwix? |
ShaopengLin
force-pushed
the
Issue#61-export-bookmark
branch
from
95fb78c
to
3f0947e
Compare
kelson42
force-pushed
the
Issue#61-export-bookmark
branch
from
3f0947e
to
18b569a
Compare
|
@ShaopengLin Thank you for the PR, I will give a try today. |
There was a problem hiding this comment.
LGTM but please use OS "Download" directory instead of current directory for the default filepicker location
@veloman-yunkan ready for code review
|
@veloman-yunkan We need urgently a code review. |
src/readinglistbar.cpp
Outdated
| void PortMenu::showEvent(QShowEvent* event) | ||
| { | ||
| Q_UNUSED(event); | ||
|
|
||
| QPoint p = this->pos(); | ||
| QRect geo = b->geometry(); | ||
| this->move(p.x()+geo.width() - 4, p.y() - b->iconSize().height()); | ||
| } |
There was a problem hiding this comment.
Can't this be solved via styling instead?
There was a problem hiding this comment.
I don't think its possible as the position seems to be hardcoded by Qt. Found no style exported to change this. We can also choose to use the default popup menu behavior instead of the subclassing.
ShaopengLin
force-pushed
the
Issue#61-export-bookmark
branch
from
42674d8
to
053f3fc
Compare
@kelson42 Should we address the above issue in this PR? |
resources/i18n/en.json
Outdated
| "zim-path": "Zim File Path" | ||
| "zim-path": "Zim File Path", | ||
| "export-reading-list": "Export reading list", | ||
| "export-reading-list-error": "An error has occured during export." |
There was a problem hiding this comment.
@kelson42 Are you fine with such an error message? I don't like that "reading list" is missing from the error text. Also no details about the error are provided, but addressing that one will require changes to libkiwix, and most likely that part won't be translated.
There was a problem hiding this comment.
Changed the message as requested and waiting for confirmation.
There was a problem hiding this comment.
@veloman-yunkan Yes, message should be "An error has occured during reading list export."
resources/i18n/en.json
Outdated
| "export-reading-list-error": "An error has occured during export." | ||
| "export-reading-list-error": "An error has occured during export.", | ||
| "import-reading-list": "Import reading list", | ||
| "import-reading-list-error": "An error has occured during import." |
There was a problem hiding this comment.
@kelson42 Are you fine with such an error message? I don't like that "reading list" is missing from the error text. Also no details about the error are provided, but addressing that one will require changes to libkiwix, and most likely that part won't be translated.
There was a problem hiding this comment.
Changed the message as requested and waiting for confirmation.
ShaopengLin
force-pushed
the
Issue#61-export-bookmark
branch
2 times, most recently
from
230c287
to
bc71850
Compare
There was a problem hiding this comment.
LGTM. Not approving yet, since there is an unanswered question.
|
@veloman-yunkan Very good point, I have open a dedicated issue as this is not trivial to fix and we can leave IMHO with this weakness #1171 |
Added Menu button to export reading list to file
Add menu button to import/append reading list from file
Document folder is the default folder to ex/import.
kelson42
force-pushed
the
Issue#61-export-bookmark
branch
from
bc71850
to
3f04646
Compare
kelson42
changed the title
Fix #61
Changes: