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.

Sign up for GitHub

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

Jump to bottom

bug: fixed router navigating to homepage on page loads #723

Merged
merged 2 commits into from
Nov 6, 2024
Merged

bug: fixed router navigating to homepage on page loads #723

merged 2 commits into from
Nov 6, 2024

Conversation

david-kalmakoff
Copy link
Contributor

Fixes #721

  • updated authenticating status when tryAuthenticating is called
  • changed a test to navigate with window location to make sure you can navigate to pages after page loads

@david-kalmakoff david-kalmakoff requested a review from a team as a code owner November 5, 2024 06:25
@codecov Codecov
Copy link

codecov bot commented Nov 5, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.42%. Comparing base (b08b5ae) to head (ad6fc49).
Report is 3 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master     #723   +/-   ##
=======================================
  Coverage   79.42%   79.42%           
=======================================
  Files          56       56           
  Lines        2639     2639           
=======================================
  Hits         2096     2096           
  Misses        452      452           
  Partials       91       91

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jmattheis
jmattheis reviewed Nov 5, 2024
View reviewed changes
ui/src/CurrentUser.ts Outdated
@@ -99,7 +99,9 @@ export class CurrentUser {
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: your commits aren't linked to your GitHub account. See the email in https://github.com/gotify/server/pull/723/commits/eaa19ddcb110296b7fd49e8589f2c5a3b22b2b55.patch

jmattheis
jmattheis approved these changes Nov 6, 2024
View reviewed changes
ui/src/CurrentUser.ts Outdated
@@ -99,7 +99,9 @@ export class CurrentUser {
};

public tryAuthenticate = async (): Promise<AxiosResponse<IUser>> => {
this.authenticating = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed this line, as I think we don't want to show the loading spinner, when we try to reconnect after there was a successful connection. I've set this.authenticating to true on variable initialization, so it should behave the same.

ui/src/CurrentUser.ts
@@ -99,7 +99,9 @@ export class CurrentUser {
};

public tryAuthenticate = async (): Promise<AxiosResponse<IUser>> => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plugin detail page threw an exception because the plugin data wasn't loaded there.

@jmattheis jmattheis merged commit 7c9d770 into gotify:master Nov 6, 2024
3 checks passed
@david-kalmakoff david-kalmakoff deleted the issue-721 branch November 6, 2024 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmattheis jmattheis jmattheis approved these changes

Assignees

@jmattheis jmattheis

Labels
in:ui
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

have router navigate to expected route on page loads
3 participants
@david-kalmakoff @jmattheis @eternal-flame-AD