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

TabbedRenderer / load icons from custom sources (from embbedded svg for example) #574

Merged
merged 5 commits into from Feb 6, 2017

Conversation

Projects
None yet
5 participants
@softlion
Contributor

softlion commented Nov 29, 2016

Description of Change

After my accepted pull request for Android, this is the same idea for iOS:
add an exensibility point so tab bar icons can be loaded from other sources, as svg or embedded resources.

I added the ability to set both the non selected and the selected icon.

Bugs Fixed

None

API Changes

Added: (in TabbedRenderer)
protected virtual Tuple<UIImage, UIImage> GetIcon(Page page);

Behavioral Changes

None

PR Checklist

  • [-] Has tests (if omitted, state reason in description)
  • [x ] Rebased on top of master at time of PR
  • Changes adhere to coding standard => i paid attention to space / tabs as you use tabs (and vs default is space)
  • [- ] Consolidate commits as makes sense => i just read the documentation on how to do this for the next pr. Github does not do this automatically for a pr ?
/// A tuple containing as item1: the unselected version of the icon, item2: the selected version of the icon (item2 can be null),
/// or null if no icon should be set.
/// </returns>
protected virtual Tuple<UIImage, UIImage> GetIcon(Page page)

This comment has been minimized.

@adrianknight89

adrianknight89 Nov 29, 2016

Contributor

This could be named GetIcons. I'd probably use IsNullOrWhitespace check. Also, I'm not sure about the summary. Most methods lack one. XF team, please tell us if you like summaries or not in an FAQ.

How are you setting Item2? If your intention is that the overriden methods set it, then the summary should be edited to reflect that.

@adrianknight89

adrianknight89 Nov 29, 2016

Contributor

This could be named GetIcons. I'd probably use IsNullOrWhitespace check. Also, I'm not sure about the summary. Most methods lack one. XF team, please tell us if you like summaries or not in an FAQ.

How are you setting Item2? If your intention is that the overriden methods set it, then the summary should be edited to reflect that.

This comment has been minimized.

@softlion

softlion Nov 29, 2016

Contributor

The name could have a "s" in this iOS version. But it has been named with the same name as its Android version to facilitate discovery.

For item2, it's my intention that it could be set by overrides, as Page does not have another icon property. You may decide that the Icon property could contain a comma separated string of 2 file icons. But it won't work on Android / WP (for now).

@softlion

softlion Nov 29, 2016

Contributor

The name could have a "s" in this iOS version. But it has been named with the same name as its Android version to facilitate discovery.

For item2, it's my intention that it could be set by overrides, as Page does not have another icon property. You may decide that the Icon property could contain a comma separated string of 2 file icons. But it won't work on Android / WP (for now).

@rmarinho rmarinho merged commit c29403a into xamarin:master Feb 6, 2017

6 checks passed

Android-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passed: 350, i…
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: Windows Debug : Tests passed: 3679, ignored: 10
Details
iOS10-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests passed: 34…
Details
iOS8-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests passed: 344…
Details
iOS9-UITests-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle 8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests passed: 346…
Details

@samhouts samhouts added D-15.4 and removed cla-already-signed labels Oct 10, 2017

@samhouts samhouts added this to the 2.3.5 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment