Skip to content

fix: dont create an actionbar if not necessary#8402

Merged
vtrifonov merged 4 commits into
NativeScript:masterfrom
Akylas:actionbar_create_if_necessary
Mar 19, 2020
Merged

fix: dont create an actionbar if not necessary#8402
vtrifonov merged 4 commits into
NativeScript:masterfrom
Akylas:actionbar_create_if_necessary

Conversation

@farfromrefug

Copy link
Copy Markdown
Collaborator

Also i changed the android check to behave like iOS

For now i kept the commented code so that you can see the change.
Also i changed the android check to behave like iOS
@cla-bot cla-bot Bot added the cla: yes label Mar 2, 2020
@vakrilov

vakrilov commented Mar 6, 2020

Copy link
Copy Markdown
Contributor

Hey @farfromrefug - this is kind of a breaking change as in Android you get an ActionBar with the name of the app by default. With your PR it is hidden by default. You can see the change in the e2e/ui-tests-app:

Screenshot from master:
Screenshot_1583484089

Screenshot from PR branch:
Screenshot_1583484050

@farfromrefug

Copy link
Copy Markdown
Collaborator Author

@vakrilov ok i thought it would be. I ll fix it !

@farfromrefug farfromrefug force-pushed the actionbar_create_if_necessary branch from 784aac4 to 7da312c Compare March 6, 2020 14:36
@farfromrefug

Copy link
Copy Markdown
Collaborator Author

@vakrilov i improved it.
Now it should be created only if needed in the sense of actionBarHidden !== true
If actionBarHidden is true then the actionBar will not be created on onLoaded but will be if actionBarHidden is changed to false.

@vtrifonov

Copy link
Copy Markdown
Contributor

@farfromrefug would you update your branch, please!

@vtrifonov vtrifonov merged commit 4a67a3b into NativeScript:master Mar 19, 2020
NathanWalker pushed a commit that referenced this pull request Aug 7, 2020
* fix: dont create an actionbar if not necessary

For now i kept the commented code so that you can see the change.
Also i changed the android check to behave like iOS

* rollback. we now try an make sure the actionbar  is created only if needed

* actually we should check for false
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants