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

#5667: [Location Sharing] - Set duration of live sharing #5671

Merged
merged 13 commits into from
Apr 5, 2022

Conversation

mnaturel
Copy link
Contributor

@mnaturel mnaturel commented Mar 31, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Adding a new bottom sheet to ask user to choose between different maximum duration for the share. It is displayed before starting the live location sharing.

Motivation and context

Closes #5667

Screenshots / GIFs

Tests

  • Ensure location sharing is enabled in "Settings -> Preferences"
  • Go to a room
  • Select location sharing by pressing the add button and then the locate icon
  • Press live location sharing option
  • Check a dismissable bottom sheet is displayed with the different duration options
  • Select one of the durations
  • Press the share button
  • Check the location sharing screen is closed
  • Check the live location service is started

Tested devices

  • Physical
  • Emulator
  • OS version(s): Android 11

Checklist

@mnaturel mnaturel requested a review from gaelledel March 31, 2022 08:10
Copy link

@gaelledel gaelledel left a comment

Choose a reason for hiding this comment

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

Slight styling updates for Light and Dark themes on the bottom action sheet are needed see specs here https://www.figma.com/file/uthEK6xuo9hSQRdWvPAre5/Location-Sharing?node-id=1240%3A92359
The Action button at the bottom should be larger in height see specs 48px

@github-actions
Copy link

github-actions bot commented Mar 31, 2022

Unit Test Results

110 files  ±0  110 suites  ±0   1m 12s ⏱️ -1s
195 tests ±0  195 ✔️ ±0  0 💤 ±0  0 ±0 
650 runs  ±0  650 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit de59f9f. ± Comparison against base commit d05d697.

♻️ This comment has been updated with latest results.

@mnaturel mnaturel force-pushed the feature/mna/PSF-673-live-loc-share-duration branch from fb86981 to cc6cb41 Compare March 31, 2022 12:45
@mnaturel
Copy link
Contributor Author

mnaturel commented Mar 31, 2022

Slight styling updates for Light and Dark themes on the bottom action sheet are needed see specs here https://www.figma.com/file/uthEK6xuo9hSQRdWvPAre5/Location-Sharing?node-id=1240%3A92359 The Action button at the bottom should be larger in height see specs 48px

I increased the height of the "Share" button (see new captures below). Is it okay?

@gaelledel
Copy link

Yes the button height is now OK - still need to change the style of the sheet themselves i.e: Dark mode seems to be using the wrong bg colour. We also only have separator under the title Action

@mnaturel mnaturel changed the title #5567: [Location Sharing] - Set duration of live sharing #5667: [Location Sharing] - Set duration of live sharing Mar 31, 2022
@mnaturel
Copy link
Contributor Author

mnaturel commented Apr 1, 2022

Yes the button height is now OK - still need to change the style of the sheet themselves i.e: Dark mode seems to be using the wrong bg colour. We also only have separator under the title Action

Sorry, I totally missed the background color. Below are the new captures after adjustments. However, I am not sure the specs are correct for the light version of the design. We are using the element_system colors for background of bottom sheets and the quinary_content color for the dividers. It matches the design of the dark version but not the one of the light version, was it intented?

Copy link

@gaelledel gaelledel left a comment

Choose a reason for hiding this comment

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

Great thanks!

@mnaturel mnaturel marked this pull request as ready for review April 1, 2022 09:33
@mnaturel mnaturel requested review from a team and fedrunov and removed request for a team April 1, 2022 09:34
Copy link
Contributor

@fedrunov fedrunov 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

@mnaturel mnaturel merged commit bebe819 into develop Apr 5, 2022
@mnaturel mnaturel deleted the feature/mna/PSF-673-live-loc-share-duration branch April 5, 2022 14:13
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.

[Location Sharing] - Set duration of live sharing
3 participants