Skip to content
This repository has been archived by the owner on Jan 6, 2025. It is now read-only.

28 shooting screen #34

Merged
merged 11 commits into from
May 7, 2024
Merged

28 shooting screen #34

merged 11 commits into from
May 7, 2024

Conversation

OtterWays
Copy link
Collaborator

I modified the UI according to the mockups provided.

It is now impossible to take photos in portrait mode and the buttons have been redone for landscape mode.

The burst mode is not yet implemented and will be in #33

@OtterWays OtterWays linked an issue Apr 24, 2024 that may be closed by this pull request
Copy link
Contributor

@Jean-BaptisteC Jean-BaptisteC left a comment

Choose a reason for hiding this comment

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

✅ Pixel 6 - Android 14

Copy link
Contributor

@luifr10 luifr10 left a comment

Choose a reason for hiding this comment

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

Changes ok for me, can you please correct the E2E tests performed during the CI ?

@OtterWays
Copy link
Collaborator Author

Changes ok for me, can you please correct the E2E tests performed during the CI ?

I have added a tooltip to the photo capture button. However, it is not an actual button but a GestureDetector. Consequently, the test 'iShouldSeeAButtonNamed($, 'Take a picture');' does not work. I don't want to delete this test, but I'm unsure how to modify it to make it pass.

@OtterWays
Copy link
Collaborator Author

@luifr10 I I am unable to fix the integration test.

@luifr10
Copy link
Contributor

luifr10 commented Apr 29, 2024

@OtterWays don't worry, I'll take a look

@luifr10
Copy link
Contributor

luifr10 commented May 2, 2024

I just realized that the behavior of the capture page has changed aha.
When running integration tests, the application defaults to portait mode, so I'm going to make 2 changes:

  • Add the sentence to change orientation in the uuv_flutter library.
  • Add a scenario that checks the presence of the message when in portrait mode.

@luifr10 luifr10 force-pushed the 28-shooting-screen branch 2 times, most recently from 379bdf7 to a0a934d Compare May 6, 2024 10:37
@luifr10 luifr10 force-pushed the 28-shooting-screen branch from a0a934d to aa1ffc9 Compare May 6, 2024 11:17
@luifr10
Copy link
Contributor

luifr10 commented May 6, 2024

Hello @OtterWays !

integration_tests are now OK, here are the changes I've made :

  • Add a scenario that checks the presence of the message when in portrait mode.
  • Change last scenario that checks landscape mode
  • Migrate router from go_router to get_it (easier for testing)
  • Wrap capture button with IconButton to have semantic role button (for accessibility we have to be careful on this point)

I also made a rebase on the main branch, it's better to do a rebase instead of a merge to update with the main branch.

If it's ok for you, we can merge then

@OtterWays OtterWays merged commit fbfdb0d into main May 7, 2024
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shooting screen.
4 participants