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

Tapping multiple bar button items #42

Conversation

immenor
Copy link
Collaborator

@immenor immenor commented May 31, 2019

I took a first pass at adding the ability to tap multiple bar button items in the navigation bar. I also added the ability to tap them based on either the title or the system item. In my case I had two icons in the rightBarButtonItems I needed to tap.

Im not sure if there are rules I should be aware of with the comments above the public functions. I tried to follow the style you had for the tapBarButtonItem(systemItem:) function. Let me know how this works.

@codecov-io
Copy link

codecov-io commented May 31, 2019

Codecov Report

Merging #42 into master will increase coverage by <.01%.
The diff coverage is 98.62%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #42      +/-   ##
=========================================
+ Coverage   97.39%   97.4%   +<.01%     
=========================================
  Files         126     126              
  Lines        5879    5966      +87     
=========================================
+ Hits         5726    5811      +85     
- Misses        153     155       +2
Impacted Files Coverage Δ
...wController/UIViewController+UIBarButtonItem.swift 100% <100%> (ø) ⬆️
...rastructure/Builders/UIViewControllerBuilder.swift 100% <100%> (ø) ⬆️
...troller/UIViewController+UIBarButtonItemSpec.swift 99.13% <98.19%> (-0.87%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2b3581...f063255. Read the comment docs.

@theextremeprogrammer theextremeprogrammer merged commit 5835cd7 into theextremeprogrammer:master Jun 7, 2019
@theextremeprogrammer
Copy link
Owner

Thanks for this PR @immenor! From a quick glance everything looked good - I may tweak a couple of things and then follow up with you afterwards. Thanks again!! 👍

@theextremeprogrammer
Copy link
Owner

@immenor please take a look at commit 526dd97 to see updates made related to this PR and let me know if you want to chat about any of these. 👍 Thanks for bringing this functionality into Succinct!

@theextremeprogrammer
Copy link
Owner

Closes #41

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