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

Always popover #17

Merged
merged 18 commits into from
Jul 30, 2015
Merged

Always popover #17

merged 18 commits into from
Jul 30, 2015

Conversation

bmichotte
Copy link
Contributor

Relative to #11

This PR force the use of popover when on iPad.

I changed the functionality with the following

If you want to present your alert in :sheet style and you are on iPad, you have to provide the :source for the popover (either a UIView or a UIBarButtonItem)

rmq.append(UIButton, :my_button).on(:tap) do |sender|
  rmq.app.alert(title: "Actions!", message: "Alert from a Popover.", 
                                   actions: [:ok, :cancel], style:sheet, source: sender)
end

You can also provide a :modal option if you want the popover to be modal. This option is only available for iOS 8+.

I have updated all tests to work on iPhone and iPad

@bmichotte
Copy link
Contributor Author

I also corrected some tests which failed in localized iDevice

@markrickert
Copy link
Collaborator

Some of those corrected tests were included in my PR #13 that's already been merged. Looks like you need a rebase here before we can merge :)

@bmichotte
Copy link
Contributor Author

I think it should be ok like this

@markrickert
Copy link
Collaborator

Loons good on my end. Maybe we should add an iPad spec build line to the travis file so we're getting full test coverage?

@markrickert
Copy link
Collaborator

Also... Wondering if we should also optionally support permittedArrowDirections for the pop over on iPad?

@GantMan
Copy link
Owner

GantMan commented Jul 30, 2015

This looks near done. I'm thinking we need mark's suggested travis ci spec change for this PR, and merge it. (Proper testing)

love the arrow_directions add!

@bmichotte
Copy link
Contributor Author

Here are the arrow_direction option ;)

@markrickert
Copy link
Collaborator

Awesome! Thanks so much for this contribution!

@GantMan
Copy link
Owner

GantMan commented Jul 30, 2015

I think I'll merge this and solve the iPad tests issue in master real quick. :)

GantMan added a commit that referenced this pull request Jul 30, 2015
@GantMan GantMan merged commit e6fb099 into GantMan:master Jul 30, 2015
@GantMan
Copy link
Owner

GantMan commented Jul 30, 2015

Tests pass on iPad Air!

https://travis-ci.org/GantMan/RedAlert/builds/73426147

@bmichotte
Copy link
Contributor Author

👍

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

Successfully merging this pull request may close these issues.

3 participants