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

Fix nav_bar_button class method #766

Conversation

jeffcarbs
Copy link
Contributor

Fixes #666

Previously nav_bar_button would set an instance variable with the arguments for the last button added. This updates the nav_bar_button method to use an array to store the arguments so you can add multiple.

Fixes jamonholmgren#666

Previously nav_bar_button would set an instance variable with the
arguments for the last button added. This updates the nav_bar_button
method to use an array to store the arguments so you can add multiple.
@jamonholmgren
Copy link
Owner

If you accidentally do left and then left again, will it record two entries in the array?

@jeffcarbs
Copy link
Contributor Author

It would put two entries into the array. When the screen is initialized it would call set_nav_bar_button which would initialize a new button for each set of arguments and set leftBarButtonItem each time. So the last one for each side would win.

How do you think this should be handled?

  • I see there's a set_nav_bar_buttons so we could theoretically use that somehow to allow someone to use multiple buttons per side.
  • We could just store these as a hash with :left, :right, :back keys instead of an array so setting multiple times still means the last one wins but we wouldn't be initializing an extra button.

I kinda think the second one makes more sense since it's probably the more common use-case, but defer to you guys. I can fix up the PR either way.

@markrickert
Copy link
Contributor

ping @jamonholmgren

@jamonholmgren
Copy link
Owner

Oh, sorry about this @jcarbo . lgtm!

jamonholmgren added a commit that referenced this pull request Mar 8, 2016
@jamonholmgren jamonholmgren merged commit f872ac8 into jamonholmgren:master Mar 8, 2016
@jamonholmgren
Copy link
Owner

I'll release a new version of ProMotion within a week.

@markrickert
Copy link
Contributor

/slackbot remind me to release a new version of ProMotion in a week

😄

@jamonholmgren
Copy link
Owner

screen shot 2016-03-07 at 5 08 55 pm

@jamonholmgren
Copy link
Owner

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