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

feat(config): expose getMode() and deprecate Config #19104

Merged
merged 4 commits into from Sep 25, 2019

Conversation

@manucorporat
Copy link
Member

commented Aug 14, 2019

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this introduce a breaking change?

  • Yes
  • No

Other information

@manucorporat manucorporat added this to In progress 🤺 in Ionic Core via automation Sep 6, 2019
@manucorporat manucorporat moved this from In progress 🤺 to Needs review 🤔 in Ionic Core Sep 6, 2019
wip
angular/src/providers/config.ts Outdated Show resolved Hide resolved
@@ -199,3 +199,16 @@ export const setupConfig = (config: IonicConfig) => {
};
return win.Ionic.config;
};

export const getMode = (): Mode => {
const win = window as any;

This comment has been minimized.

Copy link
@liamdebeasi

liamdebeasi Sep 24, 2019

Member

should this be IonicWindow as well?

Co-Authored-By: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
@manucorporat manucorporat merged commit 0f05ea4 into master Sep 25, 2019
1 check failed
1 check failed
build Workflow: build
Details
Ionic Core automation moved this from Needs review 🤔 to Done 🎉 Sep 25, 2019
@manucorporat manucorporat deleted the deprecate-config branch Sep 25, 2019
@masimplo

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2019

Hi, any info on why config.set was deprecated and what should be used instead? The deprecation message does not offer an alternative.

I am using md config irrespective of platform but want to set rippleEffect to false when the platform is iOS - which I am currently doing using config.set.

 this._config.set('rippleEffect', this._platform.is('android'));
@liamdebeasi

This comment has been minimized.

Copy link
Member

commented Oct 3, 2019

You can still set the config in app.module.ts: https://ionicframework.com/docs/utilities/config#basic-example

@masimplo

This comment has been minimized.

Copy link
Contributor

commented Oct 4, 2019

You can still set the config in app.module.ts: https://ionicframework.com/docs/utilities/config#basic-example

Yes, but from what I understand a value has to be hard-coded (either true or false for both platforms) as you don't have access to the platform service then. Unless there is a different way to differentiate between platforms that I am unaware of.

@liamdebeasi

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

You can probably just test the user agent directly in there to get the same result using a regex: /android|sink/i.test(window.navigator.userAgent)

(this is the same regex we use in the platform service)

@chriswep

This comment has been minimized.

Copy link

commented Oct 4, 2019

Hi, any info on why config.set was deprecated and what should be used instead? The deprecation message does not offer an alternative.

I also have this question. I currently change backButtonText on language change, triggered by the user on runtime. How should this be done in the future @liamdebeasi ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Ionic Core
  
Done 🎉
5 participants
You can’t perform that action at this time.