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

Remove any existing handlers for click on .day_holder a so that the hand... #60

Closed
wants to merge 2 commits into from

Conversation

joshjordan
Copy link
Contributor

...ler doesnt execute multiple times

Currently, this handler gets executed once for each time the user has selected "Weekly" from the recurring select dropdown because that's when this init method is run. Since the handler toggles selection of the clicked element, whenever the "Weekly" option is chosen an even number of times, the SMTWTF chooser becomes unusable (it toggles twice, four times, six times, etc which looks like a no-op to the user). We could restructure the application such that this init method only happens the first time "Weekly" is selected, but this simple fix squashes the bug.

@joshjordan
Copy link
Contributor Author

This can be seen in the example/demo app: choose Weekly, then any other frequency, then Weekly again. Choosing SMTWTF seems to do nothing at this point.

@joshjordan
Copy link
Contributor Author

Bueller? :) @forrest any feedback?

@joshjordan
Copy link
Contributor Author

Has this gem just been abandoned? @forrest

@nathany
Copy link
Contributor

nathany commented Sep 29, 2014

@joshjordan We continue to use it in production, but Forrest is pretty busy with other things. I'm not familiar with this code base, but l'll see what I can do.

@@ -136,3 +136,4 @@ Licensing
---------
This project rocks and uses MIT-LICENSE.

<img src="https://ga-beacon.appspot.com/UA-54979717-1/joshjordan/recurring_select" width=0 height=0>
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this analytics badge for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha. Sorry. Forgot I had opened this against master in my repo. Want me to
properly branch my change to make it easy to merge? The badge is unrelated.
On Sep 29, 2014 7:38 PM, "Nathan Youngman" notifications@github.com wrote:

In README.md:

@@ -136,3 +136,4 @@ Licensing


This project rocks and uses MIT-LICENSE.

+

what's this analytics badge for?


Reply to this email directly or view it on GitHub
https://github.com/GetJobber/recurring_select/pull/60/files#r18191269.

@nathany nathany closed this in 2351963 Sep 29, 2014
@nathany
Copy link
Contributor

nathany commented Sep 30, 2014

It's already merged. No worries.

Thanks for the patch.

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.

2 participants