Skip to content
This repository has been archived by the owner on Jun 7, 2020. It is now read-only.

[NEW] Add bottomSheet for members #2002

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

shubhsherl
Copy link
Contributor

Closes #1958

@shubhsherl shubhsherl changed the title [WIP] Add bottomSheet for members Add bottomSheet for members Feb 26, 2019
@shubhsherl
Copy link
Contributor Author

shubhsherl commented Feb 26, 2019

Please review and merge this, before merging current PR.

@shubhsherl shubhsherl changed the title Add bottomSheet for members [NEW] Add bottomSheet for members Feb 26, 2019
@@ -343,5 +343,20 @@
<!-- Report -->
<string name="submit">Submit</string> <!--TODO - Add proper translation-->
<string name="required">*required</string> <!--TODO - Add proper translation-->
<string name="report_sent">Your report has been sent!</string> <!--TODO - Add proper translation-->
<string name="report_sent">Your report has been sent!</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

This string needs to add TODO Translate.

<string name="submit">Soumettre</string>
<string name="required">*Obligatoire</string>
<string name="report_sent">Votre rapport a été envoyé!</string>
<string name="submit">Submit</string> <!-- TODO - Add proper translation -->
Copy link
Contributor

Choose a reason for hiding this comment

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

Those 3 strings are already translated above.

@@ -343,5 +343,20 @@
<!-- Report -->
<string name="submit">Submit</string> <!--TODO - Add proper translation-->
<string name="required">*required</string> <!--TODO - Add proper translation-->
<string name="report_sent">Your report has been sent!</string> <!--TODO - Add proper translation-->
<string name="report_sent">Your report has been sent!</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

This string needs to add TODO Translate.

@@ -343,5 +343,20 @@
<!-- Report -->
<string name="submit">Submit</string> <!--TODO - Add proper translation-->
<string name="required">*required</string> <!--TODO - Add proper translation-->
<string name="report_sent">Your report has been sent!</string> <!--TODO - Add proper translation-->
<string name="report_sent">Your report has been sent!</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

This string needs to add TODO Translate.

@@ -341,5 +341,20 @@
<!-- Report -->
<string name="submit">Submit</string> <!-- TODO - Add proper translation -->
<string name="required">*required</string> <!-- TODO - Add proper translation -->
<string name="report_sent">Your report has been sent!</string> <!-- TODO - Add proper translation -->
<string name="report_sent">Your report has been sent!</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

This string needs to add TODO Translate.

@philipbrito philipbrito added this to the 3.5.0 milestone Jun 18, 2019
@philipbrito
Copy link
Contributor

@shubhsherl Can you fix the conflicts here?

@shubhsherl
Copy link
Contributor Author

@shubhsherl Can you fix the conflicts here?

@filipedelimabrito Sure, I will do it.

@@ -35,8 +35,9 @@ fun newInstance(
chatRoomId: String,
chatRoomType: String,
isSubscribed: Boolean,
isFavorite: Boolean,
disableMenu: Boolean
disableMenu: Boolean,
Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I think you wrong deleted the isFavorite param here.

@@ -74,22 +79,24 @@ class ChatDetailsFragment : Fragment(), ChatDetailsView {
internal lateinit var chatRoomId: String
internal lateinit var chatRoomType: String
private var isSubscribed: Boolean = true
internal var isFavorite: Boolean = false
private var isOwner: Boolean = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the isFavorite param?

isSubscribed = bundle.getBoolean(BUNDLE_IS_SUBSCRIBED)
disableMenu = bundle.getBoolean(BUNDLE_DISABLE_MENU)
isOwner = bundle.getBoolean(BUNDLE_CHAT_ROOM_OWNER)
isMod = bundle.getBoolean(BUNDLE_CHAT_ROOM_MOD)
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the isFavorite param? We have a bundle for it.

@@ -67,23 +67,25 @@ class ChatRoomNavigator(internal val activity: ChatRoomActivity) {
chatRoomId: String,
chatRoomType: String,
isChatRoomSubscribed: Boolean,
isChatRoomFavorite: Boolean,
isMenuDisabled: Boolean
isMenuDisabled: Boolean,
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the isFavorite param?

@@ -896,7 +902,7 @@ class ChatRoomPresenter @Inject constructor(
isFavorite: Boolean,
isMenuDisabled: Boolean
) {
navigator.toChatDetails(chatRoomId, chatRoomType, isSubscribed, isFavorite, isMenuDisabled)
navigator.toChatDetails(chatRoomId, chatRoomType, isSubscribed, isMenuDisabled, isOwner(), isModerator())
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the isFavorite param?

}
notifier()
} catch (ex: RocketChatException) {
view.showMessage(ex.message!!) // TODO Remove.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not the way to go. Check how we are handling our exceptions on our codebase please. There are lots of examples demonstrating how to handle it.

notifier()
view.setMemberCount(--totalMembers)
} catch (ex: RocketChatException) {
view.showMessage(ex.message!!) // TODO Remove.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not the way to go. Check how we are handling our exceptions on our codebase please. There are lots of examples demonstrating how to handle it.

retryIO(description = "addOwner($roomId, $roomType, $userId)") { client.addOwner(roomId, roomType, userId) }
notifier()
} catch (ex: RocketChatException) {
view.showMessage(ex.message!!) // TODO Remove.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not the way to go. Check how we are handling our exceptions on our codebase please. There are lots of examples demonstrating how to handle it.

arguments = Bundle(1).apply {
putString(BUNDLE_CHAT_ROOM_ID, chatRoomId)

fun newInstance(chatRoomId: String, isOwner: Boolean, isMod: Boolean): Fragment {
Copy link
Contributor

Choose a reason for hiding this comment

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

isModerator sounds better than isMod. You have even created a function with that name on ChatRoomPresenter.kt for instance.

it,
DividerItemDecoration.HORIZONTAL
)
DividerItemDecoration(
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong indentation. Could you please fix it?

@philipbrito
Copy link
Contributor

@shubhsherl Are you going to do the necessary things here in order to merge your PR?

@shubhsherl
Copy link
Contributor Author

@shubhsherl Are you going to do the necessary things here to merge your PR?

Sorry, @filipedelimabrito I am currently involved in other projects. Unable to work on it right now.

@philipbrito
Copy link
Contributor

We'll try to fix it ourselves. Thanks for letting us know @shubhsherl

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] To add option for members in groups.
3 participants