Remove Vue.js, replaced by Qt in the library #946
Conversation
|
Current look: |
juuz0
force-pushed
the
qtlibrary
branch
6 times, most recently
from
678fca9
to
3778f5f
Compare
|
@juuz0 What is left to do? |
|
@kelson42 fully ready for review! :) |
|
@mgautierfr Can you please have a look to the feature, to the code? I will make my own user testing. |
|
@juuz0 Rebased to benefit of the latest CI/CD improvements (and the CI passing here) |
There was a problem hiding this comment.
I don't have made an intense review so far, but overall feedback is that is LGTM. There is one thing which has not been implemented and this is the spinner. There is no indication of the remote loading of the catalogue anymore, and this is a regression. Can you please re-implement this?
A smal detail, the sorting of the results works fine, but the top/down arrow is really too far from the header, see for example:
Is that possible to make it just on the right, or maybe even on the left?
Added now, thanks for informing, There was a problem with the UI blocking while reading the catalog (came from before this PR), fixed along with this.
working on it |
juuz0
force-pushed
the
qtlibrary
branch
4 times, most recently
from
c68f2fa
to
32fd05c
Compare
Put it on left Added checks for storage error (app was crashing earlier) |
The designs use kiwix logo as library icon
Added a new custom dialog box for confirmations. Currently, it popups when a book is deleted or a download has to be cancelled
When an item is hovered, colour changes to grey. Change cursor to HandPointer in treeview Open book description when item is clicked Increase description box width
Added a new widget (KiwixLoader) which is displayed when the catalog is being downloaded.
juuz0
force-pushed
the
qtlibrary
branch
4 times, most recently
from
2c2046a
to
566902c
Compare
There was a problem hiding this comment.
I think we are good. If there is things to change, we can do it later. I think it is time to merge this.
There is only one change, but you can directly fix it in the same time of the rebase-fixup.
For the last comment of @kelson42 about the missing icons, I suggest we move that in another issue and we fix it in another PR.
Put updateRemoteLibrary in a new thread so it doesn't block the main UI, particularly the loader widget
created ContentManagerHeader, inherited from QHeaderView, to implement not showing the sort indicator for first column. Changed sort indicator position to left of text
When the confirm box is initialised with isDialog=true, Ok button is showed instead of yes/no
Shows a dialog box informing the user that the file size is too large to be downloaded on current system.
The search function is now moved to filters area for more consistency Fix #554
Splitted the views into RowNode and DescriptionNode for consistency RowNode represents one row with thumbnail, name, data, size, etc. DescriptionNode shows the description
Removed the raw pointers and replaced them with std::shared_ptr
|
@kelson42 fixed the last detail. CI is complaining about a libkiwix function which was recently made public. Does it not use the latest libkiwix (current main)? |
A bug in kiwix-build make it not publishing the dependencies of kiwix-desktop. Should be fixed with kiwix/kiwix-build#633 CI here should be ok once it is merged and nightly runs. |
|
@juuz0 Does this bran really works with libkiwix 12.0.0 or does in require 12.1.0… and then the .pro file should be updated. |
|
Well it requires the current main branch of libkiwix, which is not associated to any tag yet. The correct label should be 13.0.0 whenever that is released |
|
focal passed, but kinetic is still failing for the same reason |
|
@juuz0 Kinetic can be removed. Not maintained anymore, I guess thi is the reason why it fails. |
Fixes #683
Fixes #410
Fixes #554