Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

@gaetschwartz
Copy link
Contributor

@gaetschwartz gaetschwartz commented Jul 6, 2020

Description

This PR allows to specify a suiteName when creating getting an instance of NSUserDefaults in MethodChannelSharedPreferencesStore on iOS. This is pretty much needed if the user wants its app extensions to be able to access the NSUserDefaults saved by Flutter. See Apple documentation.

Related Issues

flutter/flutter#64739

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@gaetschwartz
Copy link
Contributor Author

@stuartmorgan Sorry for the mention but I saw you recently worked on shared_prferences, would it be possible to review this PR ? It hopefully is all good but it's the first time I contribue here so I might have done mistakes.

@jimmyrogue
Copy link

@blaxou fix the conflicts plz. I need this feature too

@gaetschwartz
Copy link
Contributor Author

gaetschwartz commented Aug 27, 2020

@blaxou fix the conflicts plz. I need this feature too

@jimmyrogue I'm waiting for someone to review it and as soon as it is I will resolve the conflicts, but this PR doesn't seem to get enough attention to get reviewed.

@stuartmorgan-g
Copy link
Contributor

@blaxou I didn't see your earlier message, but I'm not a maintainer of the mobile shared_preferences plugin.

I recommend reading https://github.com/flutter/plugins/blob/master/CONTRIBUTING.md#the-review-process; note in particular that reviews are prioritized by issue priority, so not filling an issue describing this proposal is likely why it hasn't gotten attention.

@gaetschwartz
Copy link
Contributor Author

@stuartmorgan Oh you are right I completely forgot about this part. I just filed an issue on the main repo !

@jimmyrogue I posted the issue on the main repo, feel free to react to it so that it gets review faster ! flutter/flutter#64739

@makepanic
Copy link

How would a user be able to set the suiteName with the documented methods?

Am I understanding it correctly that one can't use await SharedPreferences.getInstance() but has to manually initialize a MethodChannelSharedPreferencesStore instance?

@gaetschwartz
Copy link
Contributor Author

gaetschwartz commented Apr 2, 2021

How would a user be able to set the suiteName with the documented methods?

Am I understanding it correctly that one can't use await SharedPreferences.getInstance() but has to manually initialize a MethodChannelSharedPreferencesStore instance?

 SharedPreferencesStorePlatform.instance = MethodChannelSharedPreferencesStore(suiteName: 'mySuiteName');
await SharedPreferences.getInstance(); // Always now returns the right instance with suite name specified

I need to migrate the request to null-safety though. Will see when i have time to do so.

@walsha2
Copy link

walsha2 commented Jun 17, 2021

hi @gaetschwartz! Do we know the status of this MR and getting it officially merged into shared_preferences ?

Seems like this issue has stagnated since April?

@Hixie
Copy link
Contributor

Hixie commented Oct 26, 2021

Looks like this PR is missing tests, and seems to be failing some of the existing tests.
@gaetschwartz any chance you're still looking at this PR?

@stuartmorgan-g
Copy link
Contributor

Since this is marked as a draft and hasn't been touched in a few months I'm going to close it to clean out our review queue. Please don't hesitate to submit a new PR if you decide to revisit this. Thanks!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants