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

[NEW] Implemented Webviews for Richmessages #2327

Merged
merged 7 commits into from
Jun 7, 2019
Merged

[NEW] Implemented Webviews for Richmessages #2327

merged 7 commits into from
Jun 7, 2019

Conversation

Cool-fire
Copy link
Contributor

@RocketChat/android

Changes:

Implemented Bottomsheet which displays webview for configurable heightRatio.

Screenshots or GIF for the change:

compact tall full
comp tall full

@Cool-fire
Copy link
Contributor Author

@filipedelimabrito please review this pr. I have deleted the other pr's and added all into this one

}

})
when(heightRatio){
Copy link
Contributor

Choose a reason for hiding this comment

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

What about the full here? Is it not needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@filipedelimabrito full is not needed here because only tall and compact configurations are using bottomsheet and full configuration requires a new Activity i.e webviewActivity. full doesn't need a bottomsheet.

//TODO: Open in a configurable sizable webview
//Open in a configurable sizable webview
when(temp.webViewHeightRatio){
"full" -> openFullWebPage(temp, roomId)
Copy link
Contributor

Choose a reason for hiding this comment

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

What about creating a const with this value and using it in every place on you code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@filipedelimabrito I think this property value is not known at compile time to make it const. Correct me If I am wrong.

Copy link
Contributor

@philipbrito philipbrito Jun 4, 2019

Choose a reason for hiding this comment

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

@Cool-fire It has nothing about compile time. Make it a const since it is a const.

#2160 (comment)

Copy link
Contributor Author

@Cool-fire Cool-fire Jun 6, 2019

Choose a reason for hiding this comment

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

@filipedelimabrito I am not getting why to make temp.webViewHeightRatio const. The webviewHeightRatio has values tall, full, compact. How can it be a const?. I think we can't make temp.webViewHeightRatio a const Please correct me if I am wrong @filipedelimabrito

Copy link
Contributor

Choose a reason for hiding this comment

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

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

@philipbrito philipbrito left a comment

Choose a reason for hiding this comment

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

@Cool-fire Thanks for sending this PR.

  • The missing improvement here was addressed.

@philipbrito philipbrito merged commit 4c8cbc9 into RocketChat:develop Jun 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants