dzenbot / DZNWebViewController

A simple web browser for iPhone & iPad with similar features than Safari's
https://www.cocoacontrols.com/controls/dznwebviewcontroller
MIT License
1.06k stars 181 forks source link

show info (e.g. title/URL) on navigation bar based on a NS_OPTIONS #52

Closed hesyifei closed 8 years ago

hesyifei commented 8 years ago

Show info (e.g. title/URL) on navigation bar based on a NS_OPTIONS(DZNWebInfoOnNavigationBar) instead of showPageTitleAndURL used previously

BTW I think the name of this NS_OPTIONS(DZNWebInfoOnNavigationBar) can be better. Can anyone help?

hesyifei commented 8 years ago

You could probably add the line break here instead: @"\n%@"

@dzenbot then if user uses DZNWebInfoOnNavigationBarURL only, the URL will not be centered inside navigation bar as the text is \n[URL]

dzenbot commented 8 years ago

This looks great! Thanks for doing this @eflyjason

WenchaoD commented 8 years ago

Generally looks good @dzenbot @eflyjason , but it would be better if we can have two optimizations:

  1. There should be users already use the property showPageTitleAndURL, so it would be more friendly if we declare it 'deprecated' rather than removing it from header.
  2. One difference between NS_ENUM and NS_OPTIONS is that the later supports bitwise operation, for example:
self.infoOnNavigationBar = DZNWebInfoOnNavigationBarURL | DZNWebInfoOnNavigationBarTitle;

which means both show url and title. But it turns out this is not working in this PR.

Thanks for this @eflyjason.

hesyifei commented 8 years ago

One difference between NS_ENUM and NS_OPTIONS is that the later supports bitwise operation, for example:

@WenchaoD @dzenbot I found out that other var like DZNsupportedWebActions and DZNWebNavigationTools is defined as NS_OPTIONS and I remember that work properly with |

hesyifei commented 8 years ago

which means both show url and title. But it turns out this is not working in this PR.

@WenchaoD Fixed in 1a2ca57. WVC.infoOnNavigationBar = DZNWebInfoOnNavigationBarURL | DZNWebInfoOnNavigationBarTitle; works properly now 😄

hesyifei commented 8 years ago

There should be users already use the property showPageTitleAndURL, so it would be more friendly if we declare it 'deprecated' rather than removing it from header.

bfd7414. Done :)

hesyifei commented 8 years ago

So can merge it? 😁

hesyifei commented 8 years ago

@dzenbot

The` All definition should be defined as Title | URL.

You mean DZNWebInfoOnNavigationBarAll = DZNWebInfoOnNavigationBarTitle | DZNWebInfoOnNavigationBarURL,?

hesyifei commented 8 years ago

see #56

dzenbot commented 8 years ago

Yes exactly. Why did you close the PR tho?

WenchaoD commented 8 years ago

image 😂#56