Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Show search indicator when urlBar is empty #8491

Merged
merged 1 commit into from
May 4, 2017
Merged

Show search indicator when urlBar is empty #8491

merged 1 commit into from
May 4, 2017

Conversation

cezaraugusto
Copy link
Contributor

@cezaraugusto cezaraugusto commented Apr 25, 2017 •
edited by luixxiul
Loading

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Close #8468

Test Plan:
covered by automated test

QA Steps:
empty url bar should have search icon instead of current one

@cezaraugusto cezaraugusto added feature/URLbar parity Features which should be supported in Brave since they're supported in other major browsers. QA/test-plan-specified labels Apr 25, 2017
@cezaraugusto cezaraugusto added this to the 0.15.1 milestone Apr 25, 2017
@cezaraugusto cezaraugusto self-assigned this Apr 25, 2017
@cezaraugusto cezaraugusto changed the title - Auditors: @bsclifton, @nejczdovc Show search indicator when urlBar is empty Apr 25, 2017
@srirambv srirambv mentioned this pull request Apr 26, 2017
Closed
bsclifton
bsclifton reviewed May 1, 2017
View reviewed changes
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the before (current master), I can't reproduce the issue where the search icon isn't showing. Do you have steps?
The steps shown in #8468 work fine. ex: select URL,do cmd + x for cut, notice search icon is shown (magnifying glass)

bsclifton
bsclifton reviewed May 1, 2017
View reviewed changes
Copy link
Member

@bsclifton bsclifton left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using the GIF attached to #8468, I was able to repro (keyboard immediately sets the magnifying glass; right click, shortcut does not)... but the problem is, the issue still happens (even with this patch)
urlbar

@cezaraugusto
Show search indicator when urlBar is empty
53919e6 
- Auditors: @bsclifton, @NejcZdovc
- Close #8468
- Test Plan: covered by automated test
@cezaraugusto
Copy link
Contributor Author

my bad I misunderstand STR. Updated

@cezaraugusto
Copy link
Contributor Author

btw side-effect is that this brings back search indicator when urlbar is focused, described in this comment: https://github.com/brave/browser-laptop/blob/master/app/renderer/components/navigation/urlBarIcon.js#L45

bsclifton
bsclifton approved these changes May 4, 2017
View reviewed changes
Copy link
Member

@bsclifton bsclifton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works perfect! ++

@bsclifton bsclifton merged commit 8ed5793 into brave:master May 4, 2017
@cezaraugusto cezaraugusto deleted the urlbar/8468 branch July 25, 2017 07:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bsclifton bsclifton bsclifton approved these changes

@NejcZdovc NejcZdovc Awaiting requested review from NejcZdovc

Assignees

@cezaraugusto cezaraugusto

Labels
feature/URLbar parity Features which should be supported in Brave since they're supported in other major browsers. QA/test-plan-specified
Projects
None yet
Milestone
0.15.300 (Frozen, only critical adds ...
Development

Successfully merging this pull request may close these issues.
2 participants
@cezaraugusto @bsclifton