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

Few problems #2

Open
nguyenpham opened this issue Jan 14, 2013 · 8 comments
Open

Few problems #2

nguyenpham opened this issue Jan 14, 2013 · 8 comments

Comments

@nguyenpham
Copy link

Thanks a lot for your excellent work. I love your design and the way control works.

Just few problems I see when using it for my app:

  • currentColor may be a bug. Changing that value is not affected (from your example, even I select any color, then color picker still displays only black as selected color when I come back)
  • Back button is not "standard" one for navigator bar
  • When I add whole folder "Source" into my project, it can't compile because of missing the file NEOViewController.h. It is turn out that file from example, but not Source. Have to comment out them and change NEOViewController to UIViewController. However, if I do that, currentColor can't not compile either.
  • The main class NEOColorPickerViewController is using navigator. However, all other windows such as NEOColorPickerFavoritesViewController displays as popups, but not be pushed into navigator. That may be OK for IPhone, but in IPad, when I use popover for NEOColorPickerViewController, the NEOColorPickerFavoritesViewController will popup to full screen, not in small popup size of my popover
@kartech
Copy link
Owner

kartech commented Jan 14, 2013

  1. currentColor is indeed a bug. Fixed the example and the code. The correct property is selectedColor.
  2. I'm not sure where you are getting a "back" button from. The color picker views only have "Cancel" and "Done" buttons.
  3. Fixed issue with NEOViewController. It should have been UIViewController. Fixed the related issue with currentColor.
  4. I'm not sure where you are seeing a navigator. All of the controllers are expected to be displayed as modal popups. The client code is expected to launch NEOColorPickerViewController using presentViewController. I haven't tested behavior with iPad. This is iPhone ready, not iPad ready. I need to tweak it for iPad.

@nguyenpham
Copy link
Author

Thanks for quick fix and reply.

    1. Good fix. Thanks
    1. I missed the point that your controller using Navigation Bar but not Navigation Controller. Thus my 5 cent suggestion is that you may consider to use Navigation controller too - it is more "reasonable" for using Navigation Bar (if a user sees a navigation bar, he may think it works as usual way by pushing / popping views) and can help you fix easily problems with IPad.

@kartech
Copy link
Owner

kartech commented Jan 14, 2013

I'm looking into iPad support but I don't think the use of Navigation Bar in any way implies the need to use a Navigation Controller. The navigation controller uses a navigation bar but you can definitely use a navigation bar in a modal view.

@nguyenpham
Copy link
Author

Another issue (but not a bug): the color picker works only in portrait direction. Could you support landscape too? Thus we can use it in more projects. Thanks.

@jjxtra
Copy link
Contributor

jjxtra commented Mar 16, 2013

I submitted a pull request that adds iPad support and refactors the code to use a UINavigationController which provides much more standard behavior and ease of use (i.e. no need for hacky dialogTitle property). Behavior should still be the same, but the control now works in a popover. All users will need to create it inside a navigation controller.

@nguyenpham
Copy link
Author

Hi all,

Because of due date for releasing products (Composite Photo app), I can't wait for longer and have improved myself this excellent colorpicker to:

  • support landscape mode for iPhone
  • support iPad
  • animate when saving Favorite colors
  • use NavigationController to push views
  • clean and improve the code for being easier readable and maintenance

I have just pushed all modifications / improvements on my page. There are also some videos which you can take a look how the new improvements are in actions.

@jjxtra
Copy link
Contributor

jjxtra commented Dec 11, 2013

@nguyenpham Have you merged the main branch back into your branch? I'd love to use your improvements but there were some bug fixes that I did to the main branch.

@jjxtra
Copy link
Contributor

jjxtra commented Sep 1, 2014

I've made a color picker that supports all resolutions and orientations here: https://github.com/jjxtra/DRColorPicker

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

No branches or pull requests

3 participants