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

[ChangeNotifierProvider] documentation and example incorrect. #2453

Closed
Nomeleel opened this issue Apr 10, 2023 · 5 comments
Closed

[ChangeNotifierProvider] documentation and example incorrect. #2453

Nomeleel opened this issue Apr 10, 2023 · 5 comments
Labels
documentation Improvements or additions to documentation needs triage

Comments

@Nomeleel
Copy link

Regarding the document of ChangeNotifierProvider, the content looks more like that of StateNotifierProvider.

See ChangeNotifierProvider
See StateNotifierProvider

For the TodosNotifier in the example of ChangeNotifierProvider, it can be changed to the following, refer to the case in the riverpod dev.

class TodosNotifier extends ChangeNotifier {
  final todos = <Todo>[];

  void add(Todo todo) {
    todos.add(todo);
    notifyListeners();
  }

  void remove(String todoId) {
    todos.remove(todos.firstWhere((element) => element.id == todoId));
    notifyListeners();
  }

  void toggle(String todoId) {
    for (final todo in todos) {
      if (todo.id == todoId) {
        todo.completed = !todo.completed;
        notifyListeners();
      }
    }
  }
}

Other parts trouble the author to make changes to make it more suitable for ChangeNotifierProvider.

@Nomeleel Nomeleel added documentation Improvements or additions to documentation needs triage labels Apr 10, 2023
@docweirdo
Copy link

I was about to open an issue on the same topic. In the documentation on riverpod.dev, the same code snippet includes calls to notifyListeners(). But the package docs seem incorrect to me as well.

@rrousselGit
Copy link
Owner

Oopsy. Let me fix that right now.

@rrousselGit
Copy link
Owner

Done. Will be fixed on pub.dev when the next version is published

@Nomeleel
Copy link
Author

Oopsy. Let me fix that right now.

I think it should be deleted here to avoid confused.

/// This provider is used in combination with `package:state_notifier`.

@rrousselGit
Copy link
Owner

Ah, missed this. Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation needs triage
Projects
None yet
Development

No branches or pull requests

3 participants