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

Expose pusher profile tag in advanced settings #6369

Merged
merged 2 commits into from
Jun 27, 2022

Conversation

Johennes
Copy link
Contributor

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

This makes the current pusher's profile tag visible in the advanced settings.

Motivation and context

For debugging notifications, it's helpful to know the profile tag.

Screenshots / GIFs

Before After
Screenshot_20220623_105413 Screenshot_20220623_104538

Tests

Navigate into Settings > Advanced Settings > Notification Targets

Tested devices

  • Physical
  • Emulator
  • OS version(s): 12

Checklist

Signed-off-by: Johannes Marbach <johannesm@element.io>
@Johennes Johennes force-pushed the johannes/expose-profile-tag branch from a27e3e7 to ec07355 Compare June 23, 2022 08:56
@Johennes Johennes requested review from a team, onurays and bmarty and removed request for a team June 23, 2022 08:57
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginBottom="16dp"
android:textStyle="" />
Copy link
Contributor

@ouchadam ouchadam Jun 23, 2022

Choose a reason for hiding this comment

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

the empty text style is a little strange, would have expected normal instead of blank or removing the tag entirely, might be worth cleaning up the file whilst we're in the area 🙏

Copy link
Contributor Author

@Johennes Johennes Jun 23, 2022

Choose a reason for hiding this comment

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

I toggled the text style property to bold and back to nothing selected in Android Studio and it looks like that just removes the textStyle properties altogether. Committed and pushed that change. Is that alright?

Screenshot 2022-06-23 at 12 14 26

Copy link
Contributor

Choose a reason for hiding this comment

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

yep, that's great! I'm a little surprised that the blank case had been compiling

Copy link
Contributor

@ouchadam ouchadam left a comment

Choose a reason for hiding this comment

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

tiny comment about cleaning up some xml tag usages, LGTM 💯

Signed-off-by: Johannes Marbach <johannesm@element.io>
@sonarqubecloud
Copy link

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

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.

LGTM!

@Johennes Johennes merged commit c3ae0c2 into develop Jun 27, 2022
@Johennes Johennes deleted the johannes/expose-profile-tag branch June 27, 2022 11:33
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