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

[Location Share] - Standardise "Stop" texts for live (PSG-622) #6542

Merged
merged 2 commits into from
Jul 18, 2022

Conversation

mnaturel
Copy link
Contributor

@mnaturel mnaturel commented Jul 13, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other : Enhancement

Content

Using the same text for stop button in live location share feature.
Fixing some button insets problem on some languages.

Motivation and context

Closes #6541

Screenshots / GIFs

Tests

  • Start a live location share in a room
  • Check the same text is used for stop buttons in the top status bar in the room and in the tile message
  • Check insets are good in your language
  • Press the tile message
  • Check the correct text is used for the stop button in the bottom sheet list
  • Check insets are good in your language

Tested devices

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

Checklist

@mnaturel mnaturel marked this pull request as ready for review July 13, 2022 14:43
@mnaturel mnaturel requested review from a team and onurays and removed request for a team July 13, 2022 14:43
<item name="android:gravity">center</item>
<item name="android:padding">0dp</item>
<item name="android:minWidth">0dp</item>
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary while we already set width of the view?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this is the tricky part. We want the buttons to respect certain insets on the sides. If we don't have the minWidth property, it does not take the insets into account...

<item name="android:gravity">center</item>
<item name="android:padding">0dp</item>
<item name="android:minWidth">0dp</item>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question.

<item name="android:gravity">center</item>
<item name="android:padding">0dp</item>
<item name="android:minWidth">0dp</item>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question.

Copy link
Contributor

@onurays onurays 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 one question, otherwise LGTM!

@mnaturel mnaturel force-pushed the feature/mna/lls-standardise-stop-text branch from feb66c1 to cedeb8f Compare July 18, 2022 07:44
@sonarcloud
Copy link

sonarcloud bot commented Jul 18, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mnaturel mnaturel merged commit 6b403ec into develop Jul 18, 2022
@mnaturel mnaturel deleted the feature/mna/lls-standardise-stop-text branch July 18, 2022 08:43
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 Share] - Standardise "Stop" texts for live
2 participants