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

feat(browser): Work on browser (CB-12024) #67

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

dudeofawesome
Copy link

The StatusBar plugin can support the Browser platform at least on Chrome for Android, and potentially on other browsers as well.

@cordova-qa

This comment has been minimized.

@cordova-qa

This comment has been minimized.

@cordova-qa

This comment has been minimized.

@dudeofawesome

This comment has been minimized.

@dudeofawesome
Copy link
Author

@asfgit What can I do to accelerate the process of getting this accepted?

Copy link
Contributor

@filmaj filmaj left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! A couple of things:

  • can you rebase w/ the latest master?
  • I feel like the entry line in the docs should change from "The StatusBar object provides some functions to customize the iOS and Android StatusBar." to clarify that (some) browsers are also supported.
  • can you provide me some guidance on how i can test this out on a browser platform? What browser would support this?

README.md Outdated
@@ -73,6 +73,9 @@ if (cordova.platformId == 'android') {
}
```

### Browser Support
Copy link
Contributor

Choose a reason for hiding this comment

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

May I suggest renaming this heading to "Browser Quirks", to align with the other section headings (i.e. "Android Quirks").

README.md Outdated
@@ -73,6 +73,9 @@ if (cordova.platformId == 'android') {
}
```

### Browser Support
Only a limited selection of browsers offer support for setting status bar colors.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you expand on which browsers offer this support? I feel like that's valuable information for this documentation.

@dudeofawesome
Copy link
Author

@filmaj I've rebased with master and made the requested changes to the README.

@cordova-qa

This comment has been minimized.

@cordova-qa

This comment has been minimized.

README.md Outdated
@@ -30,7 +30,7 @@ description: Control the device status bar.
StatusBar
======

> The `StatusBar` object provides some functions to customize the iOS and Android StatusBar.
> The `StatusBar` object provides some functions to customize the iOS and Android StatusBar, as well as some browser chrome.
Copy link
Member

Choose a reason for hiding this comment

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

What does "some browser chrome" exactly mean here?
"browser window on some (mobile OS system) browsers"?

Copy link
Author

Choose a reason for hiding this comment

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

It varies by browser support of course, but it typically means the tab color and the navigation bar color.

Copy link
Author

Choose a reason for hiding this comment

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

I can extend the explanation of the affected browser chrome, but I think that is better handled by the caniuse page.

@janpio
Copy link
Member

janpio commented Jul 18, 2018

Looking pretty good @dudeofawesome. I left a question, could you also maybe again rebase on master to get rid of the README conflict? Also, what are the easiest steps to try this out?

@dudeofawesome
Copy link
Author

@janpio Rebased!

To try it out, I'd pretty much just set up a regular demo project for the statusbar plugin, but add the browser platform and run it. It should just work.

@janpio
Copy link
Member

janpio commented Jul 19, 2018

Thanks.

I installed the plugin from your PR branch and adapted the default project:
https://github.com/janpio/cordova-plugin-statusbar-browserPR-test-project/

But I couldn't get it to work on iOS, both directly in Safari and also after adding the page to the homescreen and opening from there - nothing happened.

I published the browser www here:
https://janpio.github.io/cordova-plugin-statusbar-browserPR-test-project/

Did I miss anything @dudeofawesome ?

Can someone test on iOS and Windows Phone?
(Update: Test on Windows Phone Internet Explorer showed no functionality as well)

@janpio
Copy link
Member

janpio commented Nov 16, 2018

@dudeofawesome Could you have a look at my questions, I couldn't get this to work. Otherwise we would have to treat this PR as abandoned, which would be a shame.

@janpio janpio changed the title CB-12024: Browser platform support feat(browser): Work on browser (CB-12024) Jul 3, 2019
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