Differential D20022

Added focus for searchbar in history sidebar & bookmark sidebar
ClosedPublic
Actions

Authored by saishm on Mar 24 2019, 4:54 PM.

Details

Reviewers
drosca
Group Reviewers
Falkon
Commits
R875:e66835b4bfd3: Added focus for searchbar in history sidebar & bookmark sidebar
Summary

After Adding Focus , user can directly search after pressing ctrl+h in history sidebar
& ctrl+shift+b in bookmark sidebar
BUG: 393948

Diff Detail

Repository
R875 Falkon
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
saishm created this revision.Mar 24 2019, 4:54 PM
Restricted Application added a subscriber: falkon. · View Herald TranscriptMar 24 2019, 4:54 PM
saishm requested review of this revision.Mar 24 2019, 4:54 PM
saishm added a reviewer: Falkon.
saishm retitled this revision from setting focus for searchbar to setting focus for History searchbar.Mar 24 2019, 6:14 PM
saishm updated this revision to Diff 54796.Mar 25 2019, 6:29 PM
saishm retitled this revision from setting focus for History searchbar to Added focus for search in history side bar.
saishm edited the summary of this revision. (Show Details)
saishm updated this revision to Diff 54803.Mar 25 2019, 7:10 PM
saishm edited the summary of this revision. (Show Details)
saishm added a reviewer: drosca.
saishm updated this revision to Diff 54947.Mar 27 2019, 6:24 PM
saishm retitled this revision from Added focus for search in history side bar to Added focus for searchbar in history sidebar & bookmark sidebar.
saishm edited the summary of this revision. (Show Details)
SGOrava added a subscriber: SGOrava.Mar 27 2019, 6:40 PM

Also check indentation of your code.

saishm added a comment.Mar 27 2019, 7:50 PM

oh I have mixed tab n spaces that's why they were not properly shown here but shows perfectly in sublime , anyways I will correct it.

saishm updated this revision to Diff 54957.Mar 27 2019, 7:53 PM
drosca requested changes to this revision.Mar 29 2019, 5:56 PM

One unexpected thing is that it will also focus search line when opening new browser window, not just when manually opening sidebar.

src/lib/sidebar/bookmarkssidebar.cpp
53

I don't really think there needs to be new method for just this one line.

54

There is a space after }

src/lib/sidebar/bookmarkssidebar.h
40

It should be override.

This revision now requires changes to proceed.Mar 29 2019, 5:56 PM
saishm added a comment.Apr 5 2019, 3:13 PM

the focus is not changing when opening a new window it still focuses on the homepage

saishm updated this revision to Diff 55479.Apr 5 2019, 3:17 PM

removed focusSearchLine() method because it only contains one line

drosca requested changes to this revision.Apr 5 2019, 6:37 PM
drosca added inline comments.
src/lib/sidebar/bookmarkssidebar.h
40

^

This revision now requires changes to proceed.Apr 5 2019, 6:37 PM
saishm updated this revision to Diff 55754.Apr 8 2019, 4:41 PM

declared explicitly as the override

drosca accepted this revision.Apr 10 2019, 2:30 PM
This revision is now accepted and ready to land.Apr 10 2019, 2:30 PM
drosca added a comment.Apr 10 2019, 2:31 PM

Full name + email please so I can commit it for you.

saishm added a comment.Apr 10 2019, 2:46 PM
In D20022#447429, @drosca wrote:

Full name + email please so I can commit it for you.

saish mhatre
saish.mhatre26@gmail.com

Closed by commit R875:e66835b4bfd3: Added focus for searchbar in history sidebar & bookmark sidebar (authored by saishm, committed by drosca). · Explain WhyApr 10 2019, 3:14 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 55912

View Options

src/lib/sidebar/bookmarkssidebar.h

Loading...
View Options

src/lib/sidebar/bookmarkssidebar.cpp

Loading...
View Options

src/lib/sidebar/historysidebar.h

Loading...
View Options

src/lib/sidebar/historysidebar.cpp

Loading...