Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[GH-6184] - Make able to disable tabs inside the MoreViewController [iOS] #9781

Merged
merged 16 commits into from
Nov 16, 2020

Conversation

pictos
Copy link
Contributor

@pictos pictos commented Feb 29, 2020

Description of Change

Now the XF doesn't throw an exception when a TabBarItem in MoreViewController has IsEnabled = false.

Issues Resolved

API Changes

Added:

  • local function void SetEnabledForMoreController() in ShellItemRenderer.CreateTabRenderers()

Platforms Affected

  • iOS

Behavioral/Visual Changes

None

Before/After Screenshots

Before:
throws Exception

Now:
image

Testing Procedure

Create a Shell Bottom Tab application with more than 5 tabs, and set IsEnabled property to false.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@pictos pictos marked this pull request as ready for review February 29, 2020 02:19
@samhouts samhouts added p/iOS 🍎 a/shell 🐚 4.0.0 regression on 4.0.0 i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often t/bug 🐛 labels Feb 29, 2020
@samhouts samhouts added this to In Review in vCurrent (4.8.0) Feb 29, 2020
@PureWeen
Copy link
Contributor

PureWeen commented Mar 4, 2020

@pictos can you rebase this against master?

@PureWeen PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Mar 4, 2020
@pictos
Copy link
Contributor Author

pictos commented Mar 4, 2020

@PureWeen, I did the rebase.

@PureWeen
Copy link
Contributor

PureWeen commented Mar 4, 2020

@pictos it doesn't look like it was done quite correctly

if you look at the changed files
https://github.com/xamarin/Xamarin.Forms/pull/9781/files

there is a bunch of stuff there that shouldn't be there

@pictos
Copy link
Contributor Author

pictos commented Mar 4, 2020

I saw I'm looking on that 😅

@samhouts samhouts moved this from In Review to In Progress in vCurrent (4.8.0) Mar 4, 2020
@pictos
Copy link
Contributor Author

pictos commented Mar 4, 2020

@PureWeen, can you look to see if I fixed it?

@PureWeen
Copy link
Contributor

PureWeen commented Mar 4, 2020

@pictos 👍

Kicked off the UI Tests

@pictos
Copy link
Contributor Author

pictos commented Mar 5, 2020

@PureWeen this error is related to my changes?

@PureWeen
Copy link
Contributor

PureWeen commented Mar 5, 2020

@pictos looking at your branch it doesn't look like it's been rebased to master. Your master branch looks updated though

when you're on your gh-6184 try

git rebase master

And then do a force push

@pictos
Copy link
Contributor Author

pictos commented Mar 5, 2020

@PureWeen, one more try, and fingers crossed 🤞🏾

@PureWeen
Copy link
Contributor

PureWeen commented Mar 5, 2020

@pictos it looks like GitHubIssue6184_apple_iphone_11_pro_13_3_1

Is failing on all our iOS UI tests

@pictos
Copy link
Contributor Author

pictos commented Mar 6, 2020

@PureWeen, the error from CI is:
System.Exception : Error while performing Tap(Marked("Issue 5"))
----> System.Exception : Unable to tap element. Query for Marked("Issue 5") gave no results.

can I ignore the UITest? I would not able to run it on my machine, but the test is passing manually.

@samhouts samhouts added this to In Review in Shell Mar 10, 2020
@pictos
Copy link
Contributor Author

pictos commented Mar 25, 2020

@PureWeen, any update on this?

@pictos
Copy link
Contributor Author

pictos commented Apr 4, 2020

@PureWeen, I fixed the UITest, can you run it, please?

@pictos
Copy link
Contributor Author

pictos commented Apr 4, 2020

@PureWeen, the tests that failed don't seem to be related to this PR, is that correct?

@samhouts samhouts added the API-change Heads-up to reviewers that this PR may contain an API change label Jun 11, 2020
@pictos
Copy link
Contributor Author

pictos commented Nov 4, 2020

@PureWeen I did some improvements in the code and for now, I removed support for iOS14. Looks like Apple changed the API and I'll need more time to find how to access the cells inside the More UITableView. I don't know if they closed the API or changed it to another place, or if is missing something in the bindings.

@PureWeen PureWeen self-requested a review November 5, 2020 03:03
@PureWeen
Copy link
Contributor

PureWeen commented Nov 5, 2020

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@PureWeen PureWeen removed the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Nov 5, 2020
@PureWeen
Copy link
Contributor

PureWeen commented Nov 5, 2020

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@PureWeen
Copy link
Contributor

PureWeen commented Nov 6, 2020

@PureWeen
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@rmarinho rmarinho self-requested a review November 13, 2020 15:08
@rmarinho rmarinho self-assigned this Nov 13, 2020
@rmarinho rmarinho merged commit 9090bcc into xamarin:5.0.0 Nov 16, 2020
Shell automation moved this from In Review to Done Nov 16, 2020
vNext+1 (5.0.0) automation moved this from In Review to Done Nov 16, 2020
@pictos pictos deleted the gh-6184 branch July 22, 2021 02:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4.0.0 regression on 4.0.0 a/shell 🐚 API-change Heads-up to reviewers that this PR may contain an API change blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often p/iOS 🍎 t/bug 🐛
Projects
Shell
  
Done
Development

Successfully merging this pull request may close these issues.

[Bug] Invalid TabBar.Items access in iOS ShellItemRenderer
4 participants