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

Allow Custom Tabs UI to be customizable #111

Merged
merged 4 commits into from
Oct 17, 2017
Merged

Allow Custom Tabs UI to be customizable #111

merged 4 commits into from
Oct 17, 2017

Conversation

lbalmaceda
Copy link
Contributor

@lbalmaceda lbalmaceda commented Jul 18, 2017

Displaying the page title should be the default and only behavior, along with showing the current page domain. This PR doesn't add the ability to turn this off (hide title).
A builder class is provided to customize the CT UI options.

Related: #106

@lbalmaceda lbalmaceda requested review from nikolaseu and hzalaz July 18, 2017 19:52
@@ -124,6 +124,7 @@ public void run() {
Log.d(TAG, "Launching URI. Custom Tabs available: " + available);

final Intent intent = new CustomTabsIntent.Builder(session.get())
.setShowTitle(true)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be optional? I am not too keen to force all users for a single person gh issue

Copy link
Contributor Author

@lbalmaceda lbalmaceda Jul 19, 2017

Choose a reason for hiding this comment

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

Ok. But anyway the title is no different than Login in with {YOUR_APP_NAME} as is set by the hosted page (I don't know if it can be changed). I'll add in this same PR a builder/style to customize this and the toolbar color.

@twaddington
Copy link

I'd really like to see this merged. What's the current status of this change?

@lbalmaceda
Copy link
Contributor Author

@twaddington it's missing the methods to hide/show the title and change the toolbar color. I'll try to add them before eow.

@twaddington
Copy link

@lbalmaceda thanks! I personally wouldn't mind if the title just always showed and was not configurable. Not sure how others would like that though.

@lbalmaceda
Copy link
Contributor Author

@twaddington I know but we should let devs make that decision. 👍

@lbalmaceda lbalmaceda changed the title Make Chrome CustomTabs display the page title Make CustomTabs display the page title Oct 13, 2017
@lbalmaceda lbalmaceda changed the title Make CustomTabs display the page title Allow Custom Tabs UI to be customizable Oct 13, 2017
@lbalmaceda lbalmaceda requested a review from cocojoe October 13, 2017 20:59
@cocojoe
Copy link
Member

cocojoe commented Oct 17, 2017

Let's see some screenshots.

@lbalmaceda
Copy link
Contributor Author

Copy link
Member

@cocojoe cocojoe left a comment

Choose a reason for hiding this comment

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

LGTM

@lbalmaceda lbalmaceda merged commit 2565cdb into master Oct 17, 2017
@lbalmaceda lbalmaceda modified the milestones: v1-Next, 1.11.0 Oct 17, 2017
@twaddington
Copy link

Y'all are awesome. Thanks!

@lbalmaceda lbalmaceda deleted the cct-show-title branch October 19, 2017 18:04
@lbalmaceda
Copy link
Contributor Author

Please test it and give us feedback. Thanks

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.

4 participants