Closed
Conversation
jcscottiii
force-pushed
the
jcscottiii/ui-searches-v4.8.4
branch
from
2af9039 to
fa40d20
Compare
jcscottiii
force-pushed
the
jcscottiii/ui-searches-v4.8.3
branch
2 times, most recently
from
45db461 to
6353021
Compare
jcscottiii
force-pushed
the
jcscottiii/ui-searches-v4.8.4
branch
2 times, most recently
from
139c91a to
99fc73c
Compare
This change adds a new subsection to the sidebar that renders their bookmarks Added a new playwright screenshot for authenticated users Other changes: - Modify the firebase auth user context to have a null state. We need to be able to determine three states: Verified that the user is authenticated, Verified that the user is unauthenticated, and in progress. The null state takes care of the second case. - Moved login logic for playwright test to reusable helper - Address edge case for firefox when counting requests in playwright tests.
jcscottiii
force-pushed
the
jcscottiii/ui-searches-v4.8.4
branch
from
99fc73c to
ff06ce4
Compare
jcscottiii
added a commit
that referenced
this pull request
Mar 29, 2025
Background: This PR refactors data fetching in two components by turning autoRun to false. I originally observed an issue in Firefox in my original PR (#1330). The firefox browser was seeing the tasks trigger multiple times and I thought my current handling of TaskNotReadyError and DuplicateErrors/TaskNotReadyError would handle it. However, in lit task if a task triggers while there's a pending task already running, it would abort the first task. So as a result, sometimes firefox would not get any information if the original task was aborted and the subsequent tasks were returning one of the DuplicateTaskErrors/TaskNotReadyError causing no more task executions to fully succeed. This tries to simplify things by turning off autoRun and leveraging the willUpdate lit lifecycle method to precisely run a task. This allows us to not have to do checks on the task's arguments within the task itself since we control when the task exactly triggers. Main changes: - **`OverviewPage`:** The `loadingTask` is no longer set to `autoRun: true`. Instead, it is manually triggered in `willUpdate` based on changes to relevant properties and bookmark loading status. The `TaskNotReadyError` and related logic have been removed. - **`webstatus-bookmarks-service`:** - The `loadingUserSavedBookmarkByIDTask` is no longer set to `autoRun: true` and is triggered in `willUpdate`. - The task now returns an object containing both the bookmark data and the current search query. Other changes: - **`webstatus-firebase-auth-service`:** Updated the type of the `user` state to `User | null | undefined`. - Removed some redundant tests. Changed some tests to use sinon stubs - Added some more coverage too. This is a split up of #1330
This was referenced Mar 29, 2025
Collaborator
Author
github-merge-queue bot
pushed a commit
that referenced
this pull request
Mar 31, 2025
Background: This PR refactors data fetching in two components by turning autoRun to false. I originally observed an issue in Firefox in my original PR (#1330). The firefox browser was seeing the tasks trigger multiple times and I thought my current handling of TaskNotReadyError and DuplicateErrors/TaskNotReadyError would handle it. However, in lit task if a task triggers while there's a pending task already running, it would abort the first task. So as a result, sometimes firefox would not get any information if the original task was aborted and the subsequent tasks were returning one of the DuplicateTaskErrors/TaskNotReadyError causing no more task executions to fully succeed. This tries to simplify things by turning off autoRun and leveraging the willUpdate lit lifecycle method to precisely run a task. This allows us to not have to do checks on the task's arguments within the task itself since we control when the task exactly triggers. Main changes: - **`OverviewPage`:** The `loadingTask` is no longer set to `autoRun: true`. Instead, it is manually triggered in `willUpdate` based on changes to relevant properties and bookmark loading status. The `TaskNotReadyError` and related logic have been removed. - **`webstatus-bookmarks-service`:** - The `loadingUserSavedBookmarkByIDTask` is no longer set to `autoRun: true` and is triggered in `willUpdate`. - The task now returns an object containing both the bookmark data and the current search query. Other changes: - **`webstatus-firebase-auth-service`:** Updated the type of the `user` state to `User | null | undefined`. - Removed some redundant tests. Changed some tests to use sinon stubs - Added some more coverage too. This is a split up of #1330
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
1 participant
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.
This change adds a new subsection to the sidebar that renders their bookmarks
Added a new playwright screenshot for authenticated users
Other changes: