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

Audio Capture - added support for iPhone 6 and iPhone 6 Plus microphone images #76

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

Conversation

momomatteo
Copy link

Platforms affected

iOS

What does this PR do?

Added microphone images for iPhone 6 and iPhone 6 Plus screen sizes

What testing has been done on this change?

Tested on iPhone 6 and iPhone 6 Plus

Checklist

  • Reported an issue in the JIRA database
  • Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • Added automated test coverage as appropriate for this change.

@cordova-qa
Copy link

Cordova CI Build has completed successfully.

Commit - Link
Dashboard - Link

72 tests run, 0 skipped, 0 failed.

@filmaj
Copy link
Contributor

filmaj commented Apr 25, 2017

Ping @purplecabbage @shazron for a quick review. Is having these microphone images something we would want?

I am concerned that the images and configuration for this addition seem very device-specific.

@purplecabbage
Copy link
Contributor

they are both device specific, and cordova specific
I see these as something that an app developer should be providing themselves, unfortunately there is not an API for this, or a demo/doc of how a developer would do it.

The images already exist for every other device shape and size, so this isn't a new thing, just covering more new devices.

That said, I have never really agreed with the direction of providing a generic UI inside this plugin, ...
But, It is probably better to just accept this pr, and move on with another implementation some day when we find the time.

The fact that @momomatteo ;) has taken the time to send this pr should speak much louder than my personal feelings on the subject ...

@filmaj
Copy link
Contributor

filmaj commented Apr 25, 2017

Do we need an ICLA signed for this PR?

@purplecabbage
Copy link
Contributor

It is always nice to have, but not specifically required; sending a pr is proof of the intent to contribute.

In this case the graphics are really just resized versions of existing files, so this is a modification.
If brand new images were generated then we might need to insist as the ICLA also protects us from someone contributing something they don't have the rights to.

@jcesarmobile
Copy link
Member

@momomatteo can you put the issue id (CB-12562) on the title so it's linked with the original issue on JIRA?
Also, can you remove one of the #import <Cordova/CDVAvailability.h>? you duplicated it
And finally, after those changes, can you squash all the commits into one?

@jaredloson
Copy link

Leaving this here because I can't find any info elsewhere. Does anyone know why this repo is suddenly empty?

@filmaj
Copy link
Contributor

filmaj commented Apr 28, 2017

@jaredloson there is some kind of GitHub issue that is causing havoc with read-only mirrors right now. Apache Infra + GitHub are actively working on it.

If you need to access the source, you can always do so via git.apache.org/cordova-plugin-media-capture.git.

@jaredloson
Copy link

@filmaj Got it, thanks for the info!

@timbru31 timbru31 mentioned this pull request May 6, 2019
3 tasks
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.

8 participants