Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

feat: update start api to be async #558

Merged
merged 7 commits into from
Jul 4, 2024
Merged

feat: update start api to be async #558

merged 7 commits into from
Jul 4, 2024

Conversation

im-adithya
Copy link
Member

@im-adithya im-adithya commented Jul 2, 2024

Fixes getAlby/hub#27

(The video is outdated, removed messages from Setup Screen, we only show them in the Start screen now)

What Changed

Screen.Recording.2024-07-02.at.1.36.04.PM.mov

@im-adithya im-adithya requested review from bumi and rolznz July 2, 2024 08:08
@im-adithya
Copy link
Member Author

Not sure if I should do this for wails, I think we should, but there isn't no option to check if the unlock password im wails, why is that? cc @rolznz

http/http_service.go Outdated Show resolved Hide resolved
@rolznz
Copy link
Collaborator

rolznz commented Jul 2, 2024

Not sure if I should do this for wails, I think we should, but there isn't no option to check if the unlock password im wails, why is that? cc @rolznz

@im-adithya we don't have an unlock screen on Wails because it can't be accessed from the outside. So I don't think it's necessary to make any changes there.

@im-adithya
Copy link
Member Author

Makes sense, that's what i thought, leaving this for Wails 👍

frontend/src/types.ts Outdated Show resolved Hide resolved
@@ -30,6 +30,15 @@ export function SetupFinish() {
},
};

useEffect(() => {
const timer = setTimeout(() => {
if (!info?.running) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not right, info will never change inside an effect. Each time it changes, the effect will run again.

Copy link
Collaborator

Choose a reason for hiding this comment

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

you don't even need to rely on info here, when the navigation occurs, the timeout will be cleaned up

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm using info somewhere so that I don't get the unused error. Because we have to keep polling the info. I just added the eslint unused vars comment now, can you check now? I think it should be good to merge now, tested thoroughly.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@im-adithya I pushed a fix for it, please have a look

Copy link
Collaborator

@rolznz rolznz left a comment

Choose a reason for hiding this comment

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

tACK

@rolznz rolznz merged commit 46f4504 into master Jul 4, 2024
8 checks passed
@rolznz rolznz deleted the task-async-start branch July 4, 2024 14:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make API start method async
2 participants