[WIP] Fix button saying server offline after it comes back. #3213
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
psybers
changed the title
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
MikesGlitch
added
the
⚠️ Changes requested
|
@MikesGlitch What changes were requested? I see no review. |
| style?: CSSProperties; | ||
| color?: string; | ||
| }; | ||
| export function LoggedInUser({ | ||
| hideIfNoServer, | ||
| syncState, |
There was a problem hiding this comment.
Nice. This component is also used on the ManagementApp.jsx screen - can we pass the syncState from there too?
There was a problem hiding this comment.
Yes, I should be able to do this there as well.
There was a problem hiding this comment.
Ok so after looking into this area of the code, it definitely won't work.
This is making me rethink how that button works in general, and the approach used. Perhaps the button needs its own logic to determine the state instead of relying on other components.
There was a problem hiding this comment.
This is currently working in the latest master so if we merge this branch we'd be breaking it.
There was a problem hiding this comment.
Right, like I said it probably needs reworked.
MikesGlitch
removed
the
⚠️ Changes requested
|
👋 Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review. |
|
👋 Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review. |
|
👋 Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review. |
|
👋 Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review. |
|
👋 Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review. |
|
👋 Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review. |
|
👋 Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review. |
This is an alternate fix for #3128. See #3164 for another approach.