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

dialpad moved from bottom navigation tab to a separate activity #6887

Merged
merged 4 commits into from
Aug 19, 2022

Conversation

fedrunov
Copy link
Contributor

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

since we won't have bottom navigation tabs in new layout, we need to move dialpad somewhere else

Motivation and context

part of #6502
closes #6787

Screenshots / GIFs

Light Dark Black
light dark black

output-optimised

@fedrunov fedrunov requested review from a team, mnaturel, ericdecanini, amshakal and ouchadam and removed request for a team and mnaturel August 19, 2022 12:07
@@ -8,6 +8,11 @@
android:title="@string/invite_friends"
app:showAsAction="never" />

<item
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need some logic to determine if the menu item should be visible?

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! Thanks for this catch, I forgot about it

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.

small comment about the menu item visibility, otherwise LGTM (once the CI is also happy 😄 ) 💯

dismissLoadingDialog()
finish()
} catch (failure: Throwable) {
displayErrorDialog(failure)
Copy link
Member

Choose a reason for hiding this comment

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

Will the loading Dialog will be dismiss in this case?

Copy link
Member

Choose a reason for hiding this comment

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

thanks!

@fedrunov fedrunov requested review from bmarty and ouchadam August 19, 2022 12:49
@@ -107,6 +110,7 @@ class NewHomeDetailFragment @Inject constructor(
withState(viewModel) { state ->
val isRoomList = state.currentTab is HomeTab.RoomList
menu.findItem(R.id.menu_home_mark_all_as_read).isVisible = isRoomList && hasUnreadRooms
menu.findItem(R.id.menu_home_dialpad).isVisible = state.showDialPadTab
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@ElementBot
Copy link

Warnings
⚠️

vector/src/main/AndroidManifest.xml#L79 - The attribute android:allowBackup is deprecated from Android 12 and higher and may be removed in future versions. Consider adding the attribute android:dataExtractionRules specifying an @xml resource which configures cloud backups and device transfers on Android 12 and higher.

⚠️

vector/src/main/AndroidManifest.xml#L79 - The attribute android:allowBackup is deprecated from Android 12 and higher and may be removed in future versions. Consider adding the attribute android:dataExtractionRules specifying an @xml resource which configures cloud backups and device transfers on Android 12 and higher.

⚠️

vector/src/main/AndroidManifest.xml#L232 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L232 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L235 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L235 - Consider splitting data tag into multiple tags with individual attributes to avoid confusion

⚠️

vector/src/main/AndroidManifest.xml#L289 - Expecting android:screenOrientation="unspecified" or "fullSensor" for this activity so the user can use the application in any orientation and provide a great experience on Chrome OS devices

⚠️

vector/src/main/AndroidManifest.xml#L289 - Expecting android:screenOrientation="unspecified" or "fullSensor" for this activity so the user can use the application in any orientation and provide a great experience on Chrome OS devices

⚠️

vector/src/main/AndroidManifest.xml#L424 - Exported receiver does not require permission

⚠️

vector/src/main/AndroidManifest.xml#L424 - Exported receiver does not require permission

Generated by 🚫 dangerJS against 6fe401c

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@fedrunov fedrunov merged commit cc49e96 into develop Aug 19, 2022
@fedrunov fedrunov deleted the feature/nfe/app_layout_dial_pad branch August 19, 2022 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AppLayout: remove dialpad bottom tab
4 participants