This repository has been archived by the owner on Feb 20, 2023. It is now read-only.
Closes #875: Adds shortcuts #882
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ghost
assigned
sblatz
commented
Mar 6, 2019
build.gradle
Outdated
| @@ -23,6 +23,7 @@ plugins { | |||
|
|
|||
| allprojects { | |||
| repositories { | |||
| mavenLocal() | |||
There was a problem hiding this comment.
intentional until a-c fix is landed.
sblatz
force-pushed
the
875
branch
2 times, most recently
from
b54e499
to
2876cdb
Compare
pocmo
reviewed
Mar 8, 2019
sblatz
added
the
🙅 waiting
sblatz
changed the title
sblatz
force-pushed
the
875
branch
8 times, most recently
from
6f2239a
to
d4e9707
Compare
sblatz
force-pushed
the
875
branch
8 times, most recently
from
e8227b1
to
5bf876f
Compare
sblatz
commented
Mar 26, 2019
app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarUIView.kt
Outdated
Show resolved
Hide resolved
sblatz
commented
Mar 26, 2019
app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarUIView.kt
Outdated
Show resolved
Hide resolved
sblatz
commented
Mar 26, 2019
app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarUIView.kt
Outdated
Show resolved
Hide resolved
sblatz
commented
Mar 26, 2019
sblatz
removed
the
🙅 waiting
colintheshots
suggested changes
Mar 28, 2019
There was a problem hiding this comment.
For the most part, this looks good. I had a few nits. The biggest is that AwesomeBarUIView is getting a bit hefty and some methods could be broken out. We could probably use some unit tests.
I marked this PR "request changes" though mostly because it has commented code awaiting A-C changes to land. Otherwise, this is really close.
build.gradle
Outdated
| @@ -23,6 +23,7 @@ plugins { | |||
|
|
|||
| allprojects { | |||
| repositories { | |||
| mavenLocal() | |||
app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarUIView.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarUIView.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarUIView.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/mozilla/fenix/search/awesomebar/AwesomeBarUIView.kt
Outdated
Show resolved
Hide resolved
sblatz
force-pushed
the
875
branch
2 times, most recently
from
4528888
to
8304741
Compare
|
@colintheshots I refactored the logic from |
colintheshots
approved these changes
Mar 29, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'd love for this PR to be reviewed for its major content (since it's so large), and I can then change the two places with
TODOonce this a-c change lands. Alternatively, we could merge without that change and fast-follow with it tomorrow.