Guiders-JS
Guiders-JS copied to clipboard
Made a handful of changes and a couple of bug fixes
Hello. First I want to thank you for for this library. Especially for the update that moves the guiders on browser resize. That change landed as I was about to write it myself.
This is my first open source commit. I had been working for the federal government, in isolation, for 12 years. Please go easy on me. Also, I apologize for letting so many changes pool up. I'm a recent convert from Subversion. And this is the fist chance I've had to contribute code back to you.
I carefully made comments for each of the 10 commits. But the complete, unified diff of all commits doesn't really tell the story as I intended. Should these have been 10 separate pull requests?
I look forward to hearing your feedback.
Thanks Steve. They should have been logically separate pull requests, probably not 10 but not 1 either. It's not that big a deal though.
When I get some time I intend to look through all the pull requests including this and merge a bunch in.
Why did you rename dehighlightElement to removeHighlight?
Jeff Pickhardt [email protected]
On Tue, Feb 18, 2014 at 9:47 PM, Steve Farmer [email protected]:
Hello. First I want to thank you for for this library. Especially for the update that moves the guiders on browser resize. That change landed as I was about to write it myself.
This is my first open source commit. I had been working for the federal government, in isolation, for 12 years. Please go easy on me. Also, I apologize for letting so many changes pool up. I'm a recent convert from Subversion. And this is the fist chance I've had to contribute code back to you.
I carefully made comments for each of the 10 commits. But the complete, unified diff of all commits doesn't really tell the story as I intended. Should these have been 10 separate pull requests?
I look forward to hearing your feedback.
You can merge this Pull Request by running
git pull https://github.com/majorcode/Guiders-JS dev
Or view, comment on, or merge it at:
https://github.com/jeff-optimizely/Guiders-JS/pull/116 Commit Summary
- Version 2.0.0 / Merge remote-tracking branch 'origin/dev'
- Version 2.0.0 / Merge remote-tracking branch 'origin/dev'
- move component.json to bower.json
- Merge pull request #111 from aub/master
- Reduce places where the _guiders array is accessed directly by calling get() or getCurrentGuider() to find guiders.
- If next() is called on a guider that should be skipped, the return value from next() should be the result from the recursive call to next().
- Use a counter instead of a random number to automatically assign guider IDs.
- Check for myGuider === null in _attach since typeof null is 'object'.
- When adding a guider after another and setting its prev ID, reciprocate by setting the previous guider's next ID.
- Add events to observe guider prev/next navigation.
- Since we de-highlight guider.highlight, we should be highlighting it instead of guider.attachTo.
- Reset _currentGuiderID to null when all guiders are hidden.
- Replace _dehighlightElement() with _removeHighlight() and call it in key places.
- Add descriptions for guider options that were not listed in the doc.
File Changes
- M README.mdhttps://github.com/jeff-optimizely/Guiders-JS/pull/116/files#diff-0(4)
- R bower.jsonhttps://github.com/jeff-optimizely/Guiders-JS/pull/116/files#diff-1(0)
- M guiders.jshttps://github.com/jeff-optimizely/Guiders-JS/pull/116/files#diff-2(68)
Patch Links:
- https://github.com/jeff-optimizely/Guiders-JS/pull/116.patch
- https://github.com/jeff-optimizely/Guiders-JS/pull/116.diff
Reply to this email directly or view it on GitHubhttps://github.com/jeff-optimizely/Guiders-JS/pull/116 .
Hi, Jeff,
About deHighlightElement: In db7545c I rearranged places where the highlight was removed to be sure to cover some other cases. I realized that the most defensive thing to do was remove the highlight from anything with that class. So I thought a new name was best. And, since this is in internal function, the API remains unchanged.
Steve