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

Add basic UI to download a file #51

Merged
merged 6 commits into from
Jun 29, 2023
Merged

Conversation

hans-hamel
Copy link
Contributor

@hans-hamel hans-hamel commented Jun 26, 2023

Depends on branch 'state/gmsDownloadFile'. After branch 'state/gmsDownloadFile' is merged, change this PR's base branch.
Basic UI. Requires to ask permissions dynamically. Not sure if try to save the file with a extension if it doesn't have a extension.

This PR depends on #50

@HectorNarvaez HectorNarvaez added the Don't merge Can't be merged yet label Jun 27, 2023
@HectorNarvaez HectorNarvaez marked this pull request as draft June 27, 2023 19:53
@HectorNarvaez HectorNarvaez added the WIP Work In Progress label Jun 27, 2023
@HectorNarvaez HectorNarvaez marked this pull request as ready for review June 28, 2023 17:26
@HectorNarvaez HectorNarvaez added Depends on This have dependency with another PR and removed WIP Work In Progress labels Jun 28, 2023
@Anwera64
Copy link
Contributor

Anwera64 commented Jun 28, 2023

Is the "saving the file without an extension" functionality a responsibility of the library or of the sample app?

}
}

if (deniedPermissions.isNotEmpty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be an early return

Copy link
Contributor

Choose a reason for hiding this comment

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

i think in this particular case an early return will not make difference

The result code would be something like:

if (deniedPermissions.isEmpty()) {
     return
}

requestPermissions()

which is basically the same, but longer.

What do you think? @Anwera64

Base automatically changed from state/gmsDownloadFile to develop June 29, 2023 16:20
@HectorNarvaez HectorNarvaez removed Don't merge Can't be merged yet Depends on This have dependency with another PR labels Jun 29, 2023
Copy link
Contributor

@HectorNarvaez HectorNarvaez left a comment

Choose a reason for hiding this comment

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

LGTM

@HectorNarvaez HectorNarvaez merged commit 105e8fa into develop Jun 29, 2023
2 checks passed
@HectorNarvaez HectorNarvaez deleted the feature/download-ui branch June 29, 2023 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants