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

Remove Controls Navigation Controller #14976

Merged
merged 4 commits into from May 9, 2023

Conversation

PureWeen
Copy link
Member

@PureWeen PureWeen commented May 8, 2023

Description of Change

This class currently isn't used anywhere so its existence is confusing. It was incorrectly referenced inside the controls mappers. The NavigationViewHandler on iOS was never completed, the code was left there with the good intention of being completed but alas there has not been time for that yet. So, for now, we should just delete this code to reduce confusion.

  • Delete ControlsNavigationController because it's internal and not used anywhere
  • throw not implement exceptions on the iOS NavigationViewHandler so if it's accidentally used the user is directed to the correct handler. I've ran into this a number of times writing tests where I'll map the wrong handler.
  • Modify the handling of 'largertitles' and 'translucent' to use the mapper methods that were created. Those mapper methods weren't really doing anything which is why that code still worked. Those platform specifics were being handled by the code already inside NavigationRenderer
  • Add tests for largetitles and transluscent

This class currently isn't used anywhere so its existence is confusing
@PureWeen PureWeen marked this pull request as draft May 8, 2023 14:22
@PureWeen PureWeen marked this pull request as ready for review May 8, 2023 15:30
@PureWeen PureWeen enabled auto-merge (squash) May 9, 2023 14:19
@PureWeen PureWeen merged commit 1ab0c63 into main May 9, 2023
29 checks passed
@PureWeen PureWeen deleted the remove_controlsnavigationcontroller branch May 9, 2023 15:59
rmarinho pushed a commit that referenced this pull request May 30, 2023
* Removed Controls Navigation Controller

This class currently isn't used anywhere so its existence is confusing

* - use mapper methods for platform specifics

* - fix handler registration

* Update ShellTests.cs
@ghost ghost added the area/controls 🎮 Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Jul 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/controls 🎮 Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor area/navigation 🧭 NavigationPage control-pages platform/iOS 🍎
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants