Draft
Conversation
We want to expand usage of the notifications button to include subscribing to football match events. This refactor makes the button more generic and testable, to facilitate this. Firstly the component has been renamed to `NotificationsToggle`, so that it can be reused for both liveblog notifications (new content) and football notifications (match events such as goals). This component contains the logic for user events, calling Bridget, and synchronising UI with the subscription state. The UI of the button itself is handled in a new, presentational component called `ToggleButton`, which replaces `FollowNotificationsButton` and is more suitable for customisation to the new football design. `FollowNotificationsButton` will soon be deleted, as its only other usage is in the process of being deprecated in a separate change. Wrapping `NotificationsToggle` is a lightweight island component that dependency-injects the Bridget client. This allows us to use a "live" implementation of this client in production, and a mock implementation in tests and stories. As a result, this change also adds a story containing UI tests that make use of a mock Bridget client. The type describing the API of the `Notifications` Bridget client has been simplified to make dependency injection and mocking simpler. A similar type can be rolled out for the other Bridget clients in a future change.
Member
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We want to expand usage of the notifications button to include subscribing to football match events. This refactor makes the button more generic and testable, to facilitate this.
Firstly the component has been renamed to
NotificationsToggle, so that it can be reused for both liveblog notifications (new content) and football notifications (match events such as goals). This component contains the logic for user events, calling Bridget, and synchronising UI with the subscription state.The UI of the button itself is handled in a new, presentational component called
ToggleButton, which replacesFollowNotificationsButtonand is more suitable for customisation to the new football design.FollowNotificationsButtonwill soon be deleted, as its only other usage is in the process of being deprecated in a separate change (#15558).Wrapping
NotificationsToggleis a lightweight island component that dependency-injects the Bridget client. This allows us to use a "live" implementation of this client in production, and a mock implementation in tests and stories. As a result, this change also adds a story containing UI tests that make use of a mock Bridget client.The type describing the API of the
NotificationsBridget client has been simplified to make dependency injection and mocking simpler. A similar type can be rolled out for the other Bridget clients in a future change.Part of #14905.