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.

Sign up for GitHub

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

Jump to bottom

Device Manager UI review fixes (PSG-1102) #7798

Merged
merged 8 commits into from
Dec 19, 2022
Merged

Device Manager UI review fixes (PSG-1102) #7798

merged 8 commits into from
Dec 19, 2022

Conversation

onurays
Copy link
Contributor

@onurays onurays commented Dec 15, 2022 •
edited
Loading

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

We had an internal design review for the whole new DM feature. This PR fixes the found bugs.

Motivation and context

Fix design review remarks.

Screenshots / GIFs

Tests

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

@ElementBot
Copy link

Warnings
⚠️

Please add a changelog. See instructions here
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 dangerJS against 113a1d2

@onurays onurays requested a review from mnaturel December 15, 2022 17:25
@ElementBot
Copy link

Warnings
⚠️

vector/src/main/res/layout/item_other_session.xml#L7 - Attribute android:foreground has no effect on API levels lower than 23 (current min is 21)
⚠️

vector/src/main/res/layout/item_other_session.xml#L7 - Attribute android:foreground has no effect on API levels lower than 23 (current min is 21)

Generated by 🚫 dangerJS against 113a1d2

@mnaturel mnaturel force-pushed the feature/ons/fix_device_manager_ui branch from 113a1d2 to 62f742b Compare December 16, 2022 16:23
@ElementBot
Copy link

Warnings
⚠️

Please add a changelog. See instructions here
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 dangerJS against 62f742b

@ElementBot
Copy link

Warnings
⚠️

Please add a changelog. See instructions here
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 dangerJS against b29306b

@ElementBot
Copy link

Warnings
⚠️

Please add a changelog. See instructions here
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 dangerJS against 8e2d82e

@ElementBot
Copy link

Warnings
⚠️

vector/src/main/res/layout/item_other_session.xml#L7 - Attribute android:foreground has no effect on API levels lower than 23 (current min is 21)
⚠️

vector/src/main/res/layout/item_other_session.xml#L7 - Attribute android:foreground has no effect on API levels lower than 23 (current min is 21)

Generated by 🚫 dangerJS against 8e2d82e

@ElementBot
Copy link

Warnings
⚠️

Please add a changelog. See instructions here
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 dangerJS against 65d26f1

@onurays onurays marked this pull request as ready for review December 19, 2022 10:45
@ElementBot
Copy link

Warnings
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 dangerJS against 4b2353b

@ElementBot
Copy link

Warnings
⚠️

vector/src/main/res/layout/item_other_session.xml#L7 - Attribute android:foreground has no effect on API levels lower than 23 (current min is 21)
⚠️

vector/src/main/res/layout/item_other_session.xml#L7 - Attribute android:foreground has no effect on API levels lower than 23 (current min is 21)

Generated by 🚫 dangerJS against 4b2353b

@ElementBot
Copy link

Warnings
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 dangerJS against dd51293

@ElementBot
Copy link

Warnings
⚠️

vector/src/main/res/layout/item_other_session.xml#L7 - Attribute android:foreground has no effect on API levels lower than 23 (current min is 21)
⚠️

vector/src/main/res/layout/item_other_session.xml#L7 - Attribute android:foreground has no effect on API levels lower than 23 (current min is 21)

Generated by 🚫 dangerJS against dd51293

@ElementBot
Copy link

Warnings
⚠️ You seem to have made changes to views. Please consider adding screenshots.

Generated by 🚫 dangerJS against 2c0d029

mnaturel
mnaturel approved these changes Dec 19, 2022
View reviewed changes
Copy link
Contributor

@mnaturel mnaturel left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes!

ElementBot
ElementBot reviewed Dec 19, 2022
View reviewed changes
vector/src/main/res/layout/item_other_session.xml
@@ -5,9 +5,15 @@
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:foreground="?selectableItemBackground"
Copy link

Choose a reason for hiding this comment

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

  • ⚠️ Attribute android:foreground has no effect on API levels lower than 23 (current min is 21)
  • ⚠️ Attribute android:foreground has no effect on API levels lower than 23 (current min is 21)

@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

25.0% 25.0% Coverage
0.0% 0.0% Duplication

@onurays onurays merged commit e95380a into develop Dec 19, 2022
@onurays onurays deleted the feature/ons/fix_device_manager_ui branch December 19, 2022 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ElementBot ElementBot ElementBot left review comments

@mnaturel mnaturel mnaturel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@onurays @ElementBot @mnaturel