D18658: Modify search bar in the contextDrawer

aacid resigned from this revision. aacid added a comment. I haven't tried this, but if Nathan has, i guess that's good enough. the sick brain inside me says that placeholderText: documentItem && documentItem.supportsSearching ? could be shortened to placeholderText: enab

D18658: Modify search bar in the contextDrawer

ognarb updated this revision to Diff 53738. ognarb added a comment. Use string.length > 0 instead of string == "". It's faster REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18658?vs=53338&id=53738 BRANCH arcpatch-D18658 REVISION DETAIL https://phabric

D18658: Modify search bar in the contextDrawer

aacid added a comment. You didn't convert the != to !== as i asked. Would you mind doing that? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D18658 To: ognarb, #okular, ngraham, aacid Cc: yurchor, ltoscano, aacid, ngraham, okular-devel, tfella, darcyshen

D18658: Modify search bar in the contextDrawer

ognarb added a comment. @aacid It is ready to land now? REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D18658 To: ognarb, #okular, ngraham, aacid Cc: yurchor, ltoscano, aacid, ngraham, okular-devel, tfella, darcyshen

D18658: Modify search bar in the contextDrawer

ognarb updated this revision to Diff 53338. ognarb marked an inline comment as done. ognarb added a comment. Fix off by on error REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18658?vs=53268&id=53338 BRANCH arcpatch-D18658 REVISION DETAIL https://phabr

D18658: Modify search bar in the contextDrawer

aacid added inline comments. INLINE COMMENTS > CMakeLists.txt:129 > if (BUILD_OKULARKIRIGAMI) > -add_subdirectory( mobile ) > +if (${KF5_VERSION} VERSION_GREATER 5.56) > +add_subdirectory( mobile ) Shouldn't this be 5.55? REPOSITORY R223 Okular REVISION DETAIL https://phab

D18658: Modify search bar in the contextDrawer

ognarb updated this revision to Diff 53268. ognarb marked 2 inline comments as done. ognarb added a comment. Change cmake REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18658?vs=53239&id=53268 BRANCH arcpatch-D18658 REVISION DETAIL https://phabricator.

D18658: Modify search bar in the contextDrawer

aacid added inline comments. INLINE COMMENTS > CMakeLists.txt:13 > +option(BUILD_OKULARKIRIGAMI "Builds the touch-friendly frontend" ON) > +if (BUILD_OKULARKIRIGAMI) > +set(KF5_REQUIRED_VERSION "5.56.0") I would prefer if you left the required version to be 5.44.0 for everything and then in

D18658: Modify search bar in the contextDrawer

ognarb updated this revision to Diff 53239. ognarb added a comment. Use a cmake option to build okular with kde framework 5.56 and okular mobile or wit 5.44 and not okular mobile REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18658?vs=53233&id=53239 BRANCH

D18658: Modify search bar in the contextDrawer

ognarb updated this revision to Diff 53233. ognarb added a comment. Use text.length > 0 insteaf of text != "" REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18658?vs=52415&id=53233 BRANCH arcpatch-D18658 REVISION DETAIL https://phabricator.kde.org/D186

D18658: Modify search bar in the contextDrawer

aacid added inline comments. INLINE COMMENTS > Thumbnails.qml:38 > +iconName: "edit-clear" > +visible: searchField.text != "" > +onTriggered: { This needs to be !== to be 0.0004 nanoseconds faster (same in the other file) REPOSITO

D18658: Modify search bar in the contextDrawer

aacid added a comment. In D18658#424718 , @ngraham wrote: > Not to put too fine a point on it, but why should your personal convenience be more important than adopting an API that was specifically made for this use case? ifdefs where inv

D18658: Modify search bar in the contextDrawer

aacid added a comment. In D18658#424718 , @ngraham wrote: > why should your personal convenience be more important than adopting an API that was specifically made for this use case? :sigh: This is not about *my* personal convenience, this

D18658: Modify search bar in the contextDrawer

aacid added a comment. In D18658#424718 , @ngraham wrote: > But there is a real reason: to adopt a new API. And compiling KF5 is trivial with kdesrc-build these days. No, it's not REPOSITORY R223 Okular REVISION DETAIL https://phabr

D18658: Modify search bar in the contextDrawer

ngraham added a comment. Y'all need to start using `kdesrc-build`! It makes compiling the frameworks super duper easy! https://community.kde.org/Get_Involved/development#Frameworks I'm happy to help. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D18658 To: ogna

D18658: Modify search bar in the contextDrawer

yurchor added a comment. In D18658#424718 , @ngraham wrote: > But there is a real reason: to adopt a new API. And compiling KF5 is trivial with kdesrc-build these days. Not to put too fine a point on it, but why should your personal convenience

D18658: Modify search bar in the contextDrawer

ltoscano added a comment. In D18658#424718 , @ngraham wrote: > But there is a real reason: to adopt a new API. And compiling KF5 is trivial with kdesrc-build these days. Not to put too fine a point on it, but why should your personal convenience

D18658: Modify search bar in the contextDrawer

ngraham added a comment. But there is a real reason: to adopt a new API. And compiling KF5 is trivial with kdesrc-build these days. Not to put too fine a point on it, but why should your personal convenience be more important than adopting an API that was specifically made for this use case?

D18658: Modify search bar in the contextDrawer

aacid requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D18658 To: ognarb, #okular, ngraham, aacid Cc: aacid, ngraham, okular-devel, tfella, darcyshen

D18658: Modify search bar in the contextDrawer

aacid added a comment. In D18658#424047 , @ngraham wrote: > BTW @aacid, what's your objection about doing this, out of curiosity? People on rolling release distros will surely have 5.56 by the time Applications 19.04.0 is released, and once time

D18658: Modify search bar in the contextDrawer

ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. Looks great to me. Please wait to land this until @aacid or another Okular developer is okay with bumping the Frameworks dependency to 5.56. BTW @aacid, what's your objection about do

D18658: Modify search bar in the contextDrawer

ognarb added inline comments. INLINE COMMENTS > aacid wrote in CMakeLists.txt:12 > I don't like this. > > If you need a new kirigami or something make a vesion check and then enable > kirigami only if X verison is available, but i don't like the general > requirement for okular to be increased

D18658: Modify search bar in the contextDrawer

aacid added inline comments. INLINE COMMENTS > CMakeLists.txt:12 > set(QT_REQUIRED_VERSION "5.8.0") > -set(KF5_REQUIRED_VERSION "5.44.0") > I don't like this. If you need a new kirigami or something make a vesion check and then enable kirigami only if X verison is available, but i don't lik

D18658: Modify search bar in the contextDrawer

ognarb updated this revision to Diff 52415. ognarb added a comment. - Add bottomMargin to search field REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18658?vs=52407&id=52415 BRANCH arcpatch-D18658 REVISION DETAIL https://phabricator.kde.org/D18658 AFF

D18658: Modify search bar in the contextDrawer

ognarb updated this revision to Diff 52407. ognarb edited the summary of this revision. ognarb added a comment. Rebase this diff on latest state of D18716 REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D18658?vs=50684&id=

D18658: Modify search bar in the contextDrawer

ognarb added a dependency: D18716: Add a SearchField component. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D18658 To: ognarb, #okular, ngraham Cc: ngraham, okular-devel, tfella, darcyshen, aacid

D18658: Modify search bar in the contextDrawer

ognarb added a comment. > The search field shouldn't be touching the line below it; it should have as much padding on top as on bottom. Just needs to be vertically centered in its parent, probably. Ok I will change that :) > Also, instead of manufacturing your own search field, how

D18658: Modify search bar in the contextDrawer

ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. Thanks for all this great work on Okular's mobile interface! The search field shouldn't be touching the line below it; it should have as much padding on top as on bottom. Ju

D18658: Modify search bar in the contextDrawer

ognarb edited the summary of this revision. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D18658 To: ognarb, #okular Cc: okular-devel, tfella, ngraham, darcyshen, aacid

D18658: Modify search bar in the contextDrawer

ognarb edited the summary of this revision. ognarb added a reviewer: Okular. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D18658 To: ognarb, #okular Cc: okular-devel, tfella, ngraham, darcyshen, aacid

D18658: Modify search bar in the contextDrawer

ognarb created this revision. Herald added a project: Okular. Herald added a subscriber: okular-devel. ognarb requested review of this revision. REVISION SUMMARY Use a copy of Discover searchBar component, modified. The bigestchange between discover version and this version is that this versio