feat(knock) : Knock Requests Banner UI #4005

ganfra · 2024-12-06T16:28:32Z

Content

First iteration of the knock requests banner view
No data branched.

Motivation and context

Part of #3692

Screenshots / GIFs

Tests

Step 1
Step 2
Step ...

Tested devices

Physical
Emulator
OS version(s):

Checklist

Changes have been tested on an Android device or Android emulator with API 23
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request title will be used in the release note, it clearly define what will change for the user
Pull request includes screenshots or videos if containing UI changes
Pull request includes a sign off
You've made a self review of your PR

github-actions · 2024-12-06T16:35:10Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/pfYTgY

codecov · 2024-12-06T17:35:07Z

Codecov Report

Attention: Patch coverage is 85.84906% with 30 lines in your changes missing coverage. Please review.

Project coverage is 83.08%. Comparing base (eae73ac) to head (364a374).
Report is 16 commits behind head on develop.

Files with missing lines	Patch %	Lines
...quests/impl/banner/KnockRequestsBannerPresenter.kt	0.00%	13 Missing ⚠️
.../impl/banner/DefaultKnockRequestsBannerRenderer.kt	0.00%	6 Missing ⚠️
...ockrequests/impl/banner/KnockRequestsBannerView.kt	94.49%	4 Missing and 2 partials ⚠️
...krequests/impl/banner/KnockRequestsBannerEvents.kt	0.00%	2 Missing ⚠️
...ckrequests/impl/banner/KnockRequestsBannerState.kt	89.47%	1 Missing and 1 partial ⚠️
...ent/android/features/messages/impl/MessagesView.kt	93.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop    #4005    +/-   ##
=========================================
  Coverage    83.07%   83.08%            
=========================================
  Files         1815     1821     +6     
  Lines        46248    46434   +186     
  Branches      5429     5440    +11     
=========================================
+ Hits         38421    38578   +157     
- Misses        5920     5946    +26     
- Partials      1907     1910     +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bmarty

A few remarks, else LGTM

bmarty · 2024-12-08T10:02:49Z

...kotlin/io/element/android/features/knockrequests/impl/banner/KnockRequestsBannerPresenter.kt

+class KnockRequestsBannerPresenter @Inject constructor() : Presenter<KnockRequestsBannerState> {
+    @Composable
+    override fun present(): KnockRequestsBannerState {
+        var shouldShowBanner by remember { mutableStateOf(false) }


The banner will be shown at each app starts? Or will the app will store the decision to hide the banner in a later PR?

Will be stored by the sdk actually!

bmarty · 2024-12-08T10:04:06Z

...ain/kotlin/io/element/android/features/knockrequests/impl/banner/KnockRequestsBannerState.kt

+        null
+    }
+
+    val reason = if (knockRequests.size == 1) {


Could be nice to have a firstIfSingleton extension.

bmarty · 2024-12-08T10:06:16Z

...main/kotlin/io/element/android/features/knockrequests/impl/banner/KnockRequestsBannerView.kt

+                )
+                if (state.canAccept) {
+                    Button(
+                        text = "Accept",


Use stringResource please (several changes to do in this place)

Ooops, yeah forgot some strings

bmarty · 2024-12-08T10:08:57Z

...main/kotlin/io/element/android/features/knockrequests/impl/banner/KnockRequestsBannerView.kt

+                if (state.canAccept) {
+                    Button(
+                        text = "Accept",
+                        onClick = {},


Can be done later, but maybe prepare the lambda for the onClick action.

bmarty · 2024-12-08T10:14:02Z

...t/snapshots/images/features.knockrequests.impl.banner_KnockRequestsBannerView_Night_5_en.png

Show a loading wheel and disable the button here? (Loading state)

The banner will be dismissed directly in this case, I'm waiting to branch the logic

sonarqubecloud · 2024-12-09T10:35:33Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

ganfra added 4 commits December 6, 2024 13:29

knock requests : start implementing banner ui

84299fc

knock requests : update icon

e744ff0

knock requests : branch banner in room

4b16ec6

knock requests : change banner background

350a9c0

ganfra added PR-Wip For anything that isn't ready to ship and will be enabled at a later date Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. labels Dec 6, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Dec 6, 2024

ganfra marked this pull request as ready for review December 6, 2024 16:29

ganfra requested a review from a team as a code owner December 6, 2024 16:29

ganfra requested review from bmarty and removed request for a team December 6, 2024 16:29

knock requests : refine and clean banner

603deb7

ganfra force-pushed the feature/fga/requests_to_join_banner branch from e17713f to 603deb7 Compare December 6, 2024 16:53

ganfra added the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Dec 6, 2024

github-actions bot removed the Record-Screenshots Runs the 'Record Screenshots' CI job and adds a commit with any new screenshots found. label Dec 6, 2024

Update screenshots

7957973

bmarty approved these changes Dec 8, 2024

View reviewed changes

ganfra added 2 commits December 9, 2024 11:01

knock requests : use proper banner string resources

e383c7f

misc : introduce List.firstIfSingle extension

cc9365a

ganfra enabled auto-merge December 9, 2024 10:18

knock request : emit accept single request from banner

364a374

ganfra added the Run-Maestro Starts a Maestro Cloud session to run integration tests label Dec 9, 2024

github-actions bot removed the Run-Maestro Starts a Maestro Cloud session to run integration tests label Dec 9, 2024

ganfra merged commit 9dac27a into develop Dec 9, 2024
30 checks passed

ganfra deleted the feature/fga/requests_to_join_banner branch December 9, 2024 10:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(knock) : Knock Requests Banner UI #4005

feat(knock) : Knock Requests Banner UI #4005

ganfra commented Dec 6, 2024

github-actions bot commented Dec 6, 2024 •

edited

Loading

codecov bot commented Dec 6, 2024 •

edited

Loading

bmarty left a comment

bmarty Dec 8, 2024

ganfra Dec 9, 2024

bmarty Dec 8, 2024

bmarty Dec 8, 2024

ganfra Dec 9, 2024

bmarty Dec 8, 2024

bmarty Dec 8, 2024

ganfra Dec 9, 2024

sonarqubecloud bot commented Dec 9, 2024

feat(knock) : Knock Requests Banner UI #4005

feat(knock) : Knock Requests Banner UI #4005

Conversation

ganfra commented Dec 6, 2024

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

github-actions bot commented Dec 6, 2024 • edited Loading

codecov bot commented Dec 6, 2024 • edited Loading

Codecov Report

bmarty left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 9, 2024

Quality Gate passed

github-actions bot commented Dec 6, 2024 •

edited

Loading

codecov bot commented Dec 6, 2024 •

edited

Loading