Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds shortcut widget for searching #171

Closed

Conversation

amilcar-andrade
Copy link
Contributor

@amilcar-andrade amilcar-andrade commented Mar 26, 2017

Plaid already supports shortcuts for search
and for creating a designer story but it is
only available on Android 7.1

Adding a shortcut widget on the home screen
is a good enough backport for older versions

ezgif com-video-to-gif

@amilcar-andrade
Copy link
Contributor Author

amilcar-andrade commented Mar 26, 2017

@nickbutcher please review this PR. The other one (#170) was using a widget and I think it makes more sense to just use a shortcut widget.

Plaid already supports shortcuts for search
and for creating a designer story  but it is
only available on Android 7.1

Adding a shortcut widget on the home screen
is a good enough backport for older versions
@amilcar-andrade
Copy link
Contributor Author

@nickbutcher I also created different commits in this PR. So it is easier to review. Thanks in advance!

@nickbutcher
Copy link
Owner

Thanks for contributing this. I'm in two minds about merging; while I see the benefits, I'd really like to keep the app as lean as possible. As a demo app, a major goal is to demonstrate material UI and little else so that it's more understandable.

I'm considering moving the minSdk up to 23 or perhaps even beyond so this might not be needed soon.

What do you think?

@nickbutcher
Copy link
Owner

Oh and in the future, perhaps open an issue to discuss adding a new feature before PRing – i'd hate for you to spend time on something that won't get merged.

@amilcar-andrade
Copy link
Contributor Author

amilcar-andrade commented Mar 28, 2017

@nickbutcher it is your app. So you decide what to merge 😄

I just did it because it was annoying not to have the shortcuts on Android 5.0 - 7.0. I know 7.1 will give you the shortcuts but I just have 1 device running 7.1

@amilcar-andrade
Copy link
Contributor Author

@nickbutcher one last question. Do you want to keep the first and second commit?

The first one it is just creating a dimension

The second one is to hide the save button inside the SearchActivity if the user is starting the activity from the shortcut. HomeActivity handles saving the search term inside onActivityResult.

@nickbutcher
Copy link
Owner

Thanks for understanding; I think that it's best for the project not to merge this. Thanks again for your contribution and sorry that this one didn't work out.

@amilcar-andrade
Copy link
Contributor Author

No problem, good thing I first implemented the Search shortcut 😅 the post shortcut it is a little bit more complicated.

@piotrek1543
Copy link
Contributor

`As a demo app, a major goal is to demonstrate material UI and little else so that it's more understandable.

I'm considering moving the minSdk up to 23 or perhaps even beyond so this might not be needed soon.`

I wish there would be an app, which would show how to use Material Design with animations, elevation and more.. for devices with Android system version 4.x, at least from early Jelly Bean version :-D

@nickbutcher what about making branches for specific platform features like shortcuts in API 23?

@nickbutcher
Copy link
Owner

I'm reluctant to commit to creating branches as i'm not sure I've got bandwidth to keep them up to date. I am however more than happy to link out to interesting forks like I have in the README.

@amilcar-andrade
Copy link
Contributor Author

@piotrek1543 just fork this repo and create your own branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants