Skip to content

Popup: Adds a file viewer for image attachments #455

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

Merged
merged 33 commits into from
Jun 18, 2024

Conversation

eri9000
Copy link
Collaborator

@eri9000 eri9000 commented Jun 4, 2024

Adds FileViewer composable that displays Popup attachment images.

eri9000 added 17 commits May 28, 2024 11:55
# Conflicts:
#	toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/Popup.kt
#	toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/internal/element/attachment/AttachmentTile.kt
#	toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/internal/element/attachment/AttachmentsElementState.kt
#	toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/internal/fileviewer/ViewableFile.kt
@eri9000 eri9000 marked this pull request as ready for review June 4, 2024 23:08
@sorenoid sorenoid self-requested a review June 5, 2024 16:26
Copy link
Collaborator

@sorenoid sorenoid left a comment

Choose a reason for hiding this comment

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

I have not run this yet, but have some suggestions that should be considered.

@eri9000 eri9000 requested a review from sorenoid June 5, 2024 23:43
@eri9000 eri9000 requested a review from kaushikrw June 13, 2024 21:38
Copy link
Collaborator

@kaushikrw kaushikrw left a comment

Choose a reason for hiding this comment

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

Looks good! Just some minor nitpicks that don't need a re-review.

Copy link
Collaborator

@sorenoid sorenoid left a comment

Choose a reason for hiding this comment

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

overall looks great. Strings need to be added to resources, a few odds and ends but overall well done.



@Composable
internal fun ImageViewer(path: String) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does ImageViewer do anything besides just wrap the call to AsyncImage. It doesn't seem necessary.

@eri9000 eri9000 requested a review from sorenoid June 17, 2024 21:21
@eri9000 eri9000 merged commit 2a1413c into feature-branches/popup Jun 18, 2024
@eri9000 eri9000 deleted the Erick/FileViewer_Image branch June 18, 2024 17:08
eri9000 added a commit that referenced this pull request Jun 18, 2024
commit 2a1413c
Author: Erick Lopez Solis <[email protected]>
Date:   Tue Jun 18 10:08:20 2024 -0700

    `Popup:` Adds a file viewer for image attachments (#455)

commit 2223e8e
Author: Erick Lopez Solis <[email protected]>
Date:   Fri Jun 14 10:11:06 2024 -0700

    `Popup:` Fixes TextPopupElement composable shifting in size when coming back into visibility (#461)
sorenoid added a commit that referenced this pull request Jul 11, 2024
* `Popup`: component and app (#406)

* add Popup component and microapp

* add bottom sheet and api file.

* add READMEs

* address review feedback

* change name in android manifest that were changed in previous commit

* Update microapps/PopupApp/README.md

Co-authored-by: Erick Lopez Solis <[email protected]>

* Update toolkit/popup/README.md

Co-authored-by: Erick Lopez Solis <[email protected]>

* add initialization support to the Popup composable.

---------

Co-authored-by: Soren Roth <[email protected]>
Co-authored-by: Erick Lopez Solis <[email protected]>

* `Popup`: Adds TextPopupElement Composable (#413)

* `Popup`: state collection (#420)

* pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose)

* Address warnings from version upgrades.
pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose).

* preliminary setup of state collection.

* add initialization support to the Popup composable.

* workaround for known issue in gradle task graph progress. opt in for experimental coroutine API

* view model added to the Popup App.

* move files around. make state objects Parcelable.

* ensure states are not created until expressions are evaluated.

* Update toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/Popup.kt

Co-authored-by: Erick Lopez Solis <[email protected]>

* remove Parcelable super class from base state class

* remove comment

---------

Co-authored-by: Soren Roth <[email protected]>
Co-authored-by: Erick Lopez Solis <[email protected]>

* update expandable card (#426)

* pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose)

* Address warnings from version upgrades.
pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose).

* preliminary setup of state collection.

* add initialization support to the Popup composable.

* workaround for known issue in gradle task graph progress. opt in for experimental coroutine API

* view model added to the Popup App.

* move files around. make state objects Parcelable.

* ensure states are not created until expressions are evaluated.

* Update toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/Popup.kt

Co-authored-by: Erick Lopez Solis <[email protected]>

* remove Parcelable super class from base state class

* make expandable card more like field maps.

* add defaults

* address PR feedback

* fix formatting

* default to expandable card expanded

---------

Co-authored-by: Soren Roth <[email protected]>
Co-authored-by: Erick Lopez Solis <[email protected]>

* `Popup` attachment element (#430)

* pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose)

* Address warnings from version upgrades.
pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose).

* preliminary setup of state collection.

* add initialization support to the Popup composable.

* workaround for known issue in gradle task graph progress. opt in for experimental coroutine API

* view model added to the Popup App.

* move files around. make state objects Parcelable.

* ensure states are not created until expressions are evaluated.

* attachment initial support

* Revert "attachment initial support"

This reverts commit c888b6b.

* attachment initial support

* Atachments hooked up to state collection and rendering in the Popup.

* remove file from other branch

* Update toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/Popup.kt

Co-authored-by: Erick Lopez Solis <[email protected]>

* remove Parcelable super class from base state class

* make expandable card more like field maps.

* add defaults

* address PR feedback

* fix formatting

* default to expandable card expanded

* merge in expandable card refactors

* support document and other content types.

* add all shapes sizes and color to theme defaults object

* remove newlines

* remove element count, add toggleable parameter to expandable card

---------

Co-authored-by: Soren Roth <[email protected]>
Co-authored-by: Erick Lopez Solis <[email protected]>

* `Popup:` Adds FieldsPopupElement composable (#428)

* Adds ViewableFile implementation  (#445)

* popup media element and media image thumbnail support (#449)

* Media element images wip

* Media element with support for images

* use colors from defaults

* fix capitalization

---------

Co-authored-by: Soren Roth <[email protected]>

* `Popup`: chart images (#452)

* Media element images wip

* Media element with support for images

* chart image support, dark mode support for media.

* update SDK dependency for Popups

* tweak padding of charts vs images

---------

Co-authored-by: Soren Roth <[email protected]>

* `Popup:` Fixes TextPopupElement composable shifting in size when coming back into visibility (#461)

* `Popup:` Adds a file viewer for image attachments (#455)

* `Popup:` Adds support for viewing videos and audio in FileViewer (#464)

* Adds support for opening files in the system file viewer (#472)

* chart support for AsyncImage from saved files (#475)

* chart support for AsyncImage from saved files

* remove unused lambda. update comment

---------

Co-authored-by: Soren Roth <[email protected]>

* attachment instance id workaround (#483)

* chart support for AsyncImage from saved files

* remove unused lambda. update comment

* d

* copy attachments on first fetch and pass down into the composition

---------

Co-authored-by: Soren Roth <[email protected]>

* `Popup:` Adds support for viewing media in file viewer (#481)

* popup dynamic entity (#492)

* wip

* Basic support for DynamicEntity in Popups

* minor formatting

* merge in media FileViewer

* only animate the fields element when the dynamic entity pulses -- for now.

* remove unused named lambda parameter. remove newline

---------

Co-authored-by: Soren Roth <[email protected]>

* dont recreate the media image if it has already been saved to disk (#497)

Co-authored-by: Soren Roth <[email protected]>

* remove space. add gradle stuff for tests

* add Popup to t9manifest

* update api file

* some small fixes (#508)

Co-authored-by: Soren Roth <[email protected]>

* set default map to fourteeners.

---------

Co-authored-by: Soren Roth <[email protected]>
Co-authored-by: Erick Lopez Solis <[email protected]>
sorenoid added a commit that referenced this pull request Jul 16, 2024
* `Popup`: component and app (#406)

* add Popup component and microapp

* add bottom sheet and api file.

* add READMEs

* address review feedback

* change name in android manifest that were changed in previous commit

* Update microapps/PopupApp/README.md

Co-authored-by: Erick Lopez Solis <[email protected]>

* Update toolkit/popup/README.md

Co-authored-by: Erick Lopez Solis <[email protected]>

* add initialization support to the Popup composable.

---------

Co-authored-by: Soren Roth <[email protected]>
Co-authored-by: Erick Lopez Solis <[email protected]>

* `Popup`: Adds TextPopupElement Composable (#413)

* `Popup`: state collection (#420)

* pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose)

* Address warnings from version upgrades.
pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose).

* preliminary setup of state collection.

* add initialization support to the Popup composable.

* workaround for known issue in gradle task graph progress. opt in for experimental coroutine API

* view model added to the Popup App.

* move files around. make state objects Parcelable.

* ensure states are not created until expressions are evaluated.

* Update toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/Popup.kt

Co-authored-by: Erick Lopez Solis <[email protected]>

* remove Parcelable super class from base state class

* remove comment

---------

Co-authored-by: Soren Roth <[email protected]>
Co-authored-by: Erick Lopez Solis <[email protected]>

* update expandable card (#426)

* pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose)

* Address warnings from version upgrades.
pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose).

* preliminary setup of state collection.

* add initialization support to the Popup composable.

* workaround for known issue in gradle task graph progress. opt in for experimental coroutine API

* view model added to the Popup App.

* move files around. make state objects Parcelable.

* ensure states are not created until expressions are evaluated.

* Update toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/Popup.kt

Co-authored-by: Erick Lopez Solis <[email protected]>

* remove Parcelable super class from base state class

* make expandable card more like field maps.

* add defaults

* address PR feedback

* fix formatting

* default to expandable card expanded

---------

Co-authored-by: Soren Roth <[email protected]>
Co-authored-by: Erick Lopez Solis <[email protected]>

* `Popup` attachment element (#430)

* pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose)

* Address warnings from version upgrades.
pull compose bom into androidTest and test Implementations when needed. Add missing version for artifact not covered by compose bom (androidx-activity:activity-compose).

* preliminary setup of state collection.

* add initialization support to the Popup composable.

* workaround for known issue in gradle task graph progress. opt in for experimental coroutine API

* view model added to the Popup App.

* move files around. make state objects Parcelable.

* ensure states are not created until expressions are evaluated.

* attachment initial support

* Revert "attachment initial support"

This reverts commit c888b6b.

* attachment initial support

* Atachments hooked up to state collection and rendering in the Popup.

* remove file from other branch

* Update toolkit/popup/src/main/java/com/arcgismaps/toolkit/popup/Popup.kt

Co-authored-by: Erick Lopez Solis <[email protected]>

* remove Parcelable super class from base state class

* make expandable card more like field maps.

* add defaults

* address PR feedback

* fix formatting

* default to expandable card expanded

* merge in expandable card refactors

* support document and other content types.

* add all shapes sizes and color to theme defaults object

* remove newlines

* remove element count, add toggleable parameter to expandable card

---------

Co-authored-by: Soren Roth <[email protected]>
Co-authored-by: Erick Lopez Solis <[email protected]>

* `Popup:` Adds FieldsPopupElement composable (#428)

* Adds ViewableFile implementation  (#445)

* popup media element and media image thumbnail support (#449)

* Media element images wip

* Media element with support for images

* use colors from defaults

* fix capitalization

---------

Co-authored-by: Soren Roth <[email protected]>

* `Popup`: chart images (#452)

* Media element images wip

* Media element with support for images

* chart image support, dark mode support for media.

* update SDK dependency for Popups

* tweak padding of charts vs images

---------

Co-authored-by: Soren Roth <[email protected]>

* `Popup:` Fixes TextPopupElement composable shifting in size when coming back into visibility (#461)

* `Popup:` Adds a file viewer for image attachments (#455)

* `Popup:` Adds support for viewing videos and audio in FileViewer (#464)

* Adds support for opening files in the system file viewer (#472)

* chart support for AsyncImage from saved files (#475)

* chart support for AsyncImage from saved files

* remove unused lambda. update comment

---------

Co-authored-by: Soren Roth <[email protected]>

* attachment instance id workaround (#483)

* chart support for AsyncImage from saved files

* remove unused lambda. update comment

* d

* copy attachments on first fetch and pass down into the composition

---------

Co-authored-by: Soren Roth <[email protected]>

* `Popup:` Adds support for viewing media in file viewer (#481)

* popup dynamic entity (#492)

* wip

* Basic support for DynamicEntity in Popups

* minor formatting

* merge in media FileViewer

* only animate the fields element when the dynamic entity pulses -- for now.

* remove unused named lambda parameter. remove newline

---------

Co-authored-by: Soren Roth <[email protected]>

* dont recreate the media image if it has already been saved to disk (#497)

Co-authored-by: Soren Roth <[email protected]>

* remove space. add gradle stuff for tests

* add Popup to t9manifest

* update api file

* some small fixes (#508)

Co-authored-by: Soren Roth <[email protected]>

* set default map to fourteeners.

* popup style completeness (#509)

* provide expandable card text color

* Set the color of the Popup title text

---------

Co-authored-by: Soren Roth <[email protected]>

* update dynamic charts and images (#515)

* consistent animation. todo: push animation down to components

* push animation down to components. refactor PopupMediaState creation to use factories.

* add refreshInterval support.

* reset map to fourteeners

* feedback. property name, cleanup

---------

Co-authored-by: Soren Roth <[email protected]>

---------

Co-authored-by: Soren Roth <[email protected]>
Co-authored-by: Erick Lopez Solis <[email protected]>
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.

3 participants