-
Notifications
You must be signed in to change notification settings - Fork 1.3k
For #5567: Removes search shortcuts button #5739
Conversation
@brampitoyo please verify this matches your expected mocks :) |
3a339e0
to
ea273ee
Compare
Codecov Report
@@ Coverage Diff @@
## master #5739 +/- ##
============================================
- Coverage 14.46% 14.45% -0.01%
- Complexity 322 323 +1
============================================
Files 255 256 +1
Lines 10490 10505 +15
Branches 1526 1529 +3
============================================
+ Hits 1517 1519 +2
- Misses 8851 8864 +13
Partials 122 122
Continue to review full report at Codecov.
|
ea273ee
to
1f68e05
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sblatz This looks good! Thanks for suggesting an awesome, easy to build idea that solves our problem.
searchStore.dispatch(SearchFragmentAction.ShowSearchShortcutEnginePicker(!isOpen)) | ||
|
||
if (isOpen) { | ||
requireComponents.analytics.metrics.track(Event.SearchShortcutMenuClosed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove these telemetry items? (Should not using these events break the build on telemetry?)
android:id="@+id/searchShortcutsButton" | ||
style="@style/search_pill" | ||
android:drawableStart="@drawable/ic_search" | ||
android:textOff="@string/search_shortcuts_button" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still need this string?
1f68e05
to
73fad7c
Compare
Pull Request checklist
After merge
To download an APK when reviewing a PR: