Closed kenthumphries closed 10 years ago
kenthumphries
commented
10 years ago A change I forgot to mention in the initial pull request message (though I did include it in commit 96682ff) was letting the zero point after a full rotation (i.e. 359 then 0 degrees rotation) equal minValue, not maxValue. I could see this being a breaking change for some, so I'd be happy to change it back to maxValue if required.
eliotfowler
commented
10 years ago @kenthumphries Thanks a ton for your contribution. I will look over the changes in the next couple of days and merge it in if it looks good.
kenthumphries
commented
10 years ago Hey Eliot,
Glad that you were happy with the changes! One thing I didn't do is update the Readme file. Would you like me to have a go - or do you prefer to maintain this file yourself?
eliotfowler
commented
10 years ago If you've got time, that would be greatly appreciated. If not, that was something I had planned for this weekend.
Sent from my iPhone
On Jul 16, 2014, at 8:02 AM, Kent Humphries notifications@github.com wrote:
Hey Eliot,
Glad that you were happy with the changes! One thing I didn't do is update the Readme file. Would you like me to have a go - or do you prefer to maintain this file yourself?
— Reply to this email directly or view it on GitHub.
kenthumphries
commented
10 years ago Cool - we'll see who gets there first. You will probably beat me to it. :)
On Wed, Jul 16, 2014 at 1:08 PM, Eliot notifications@github.com wrote:
If you've got time, that would be greatly appreciated. If not, that was something I had planned for this weekend.
Sent from my iPhone
On Jul 16, 2014, at 8:02 AM, Kent Humphries notifications@github.com wrote:
Hey Eliot,
Glad that you were happy with the changes! One thing I didn't do is update the Readme file. Would you like me to have a go - or do you prefer to maintain this file yourself?
— Reply to this email directly or view it on GitHub.
— Reply to this email directly or view it on GitHub https://github.com/eliotfowler/EFCircularSlider/pull/23#issuecomment-49156070 .
Hi Eliot,
I started using your EFCircularSlider component - it's terrific, thank you! Something I'd been keeping an eye out for since many months. It helped me kick start a fun app using the slider to change the time.
As I started using your component I realised that I wanted to make it work with AutoLayout. Then I realised that I wanted to better understand how the class was working. As I result I took on a fairly major refactoring process. I even split out all the maths into a separate class to make the EFCircularSlider class something that didn't need to (firsthand) know about trigonometry!
I haven't done any open source work in the past, so when I started off I didn't follow the usual fork, modify, pull request path. Instead I did a whole lot of work on a private repository. I've FINALLY had some time on my hands, so wanted to Do The Right Thing and offer my changes back to you & the community. However my refactoring work was now out of date by about 7 commits.
As such I tried to recreate my work on a fork with the following atomic commits:
507d46c move existing EFCircularSlider into a 'legacy' placeholder class 96682ff dropped in my refactored EFCircularSlider & accompanying EFCircularTrig d4590a5 Removed half finished 'outer' label feature that I started but didn't have time to finish 643a112 Ported commit 94dd521 made by ciotto on main branch after I did my refactoring 699de73 Ported commits b390cf5, 7661b21, 28db7f5, bb9f55a (where necessary) made by fatuhoku & ciotto on main branch after I did my refactoring dd23c8c Add demo to show how it now (also) works with AutoLayout 2c2409c Removed 'legacy' EFCircularSlider class
It would be great if you would accept these changes into the main branch. I tried to preserve all existing functionality, except for the line displacement property as described in my commit 699de73 - I don't think it's necessary with the updated AutoLayout/initWithRadius property. I appreciate it's a big change though, and that my preferred style may not match your own. If there's something that confuses you....well then this refactoring has failed it's main mission of improving clarity & modifiability!
Please let me know what you think.
Thanks Kent