8000 Fix menubar not ready by SRWieZ · Pull Request #453 · NativePHP/laravel · GitHub
[go: up one dir, main page]
Skip to content

Fix menubar not ready #453

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
Merged
merged 5 commits into from
Dec 29, 2024
Merged

Fix menubar not ready #453

merged 5 commits into from
Dec 29, 2024

Conversation

SRWieZ
Copy link
Member
@SRWieZ SRWieZ commented Dec 26, 2024 •
edited
Loading

The creation of MenuBar is actually asynchronous, which is not practical in PHP since we cannot use await if necessary.

This gives the ability to call create ourselves instead of __destruct.

Fix the following situation in the service provider:

MenuBar::create()
    ->route('dashboard');

// Start the background task that updates the menu bar label every second
ChildProcess::artisan(
    cmd: 'app:background-task',
    alias: 'background-task',
    persistent: true,
);

// A temporary menubar is created before the real one 
// and the menubar label flickers once because create() is called after

With this PR, we can't take control and wait for the menubar to be ready by:

MenuBar::create()
    ->route('dashboard')
    ->now();

The repetition of create is not ideal. We can change the name of the latter without it being a breaking change.

Goes with: NativePHP/electron#150

@SRWieZ SRWieZ mentioned this pull request Dec 26, 2024
Merged
@simonhamp
Copy link
Member
simonhamp commented Dec 26, 2024 •
edited
Loading

Yeh I bumped into this too and opted to have my MenuBar's controller/Livewire component be the thing that spawns the ChildProcess as the timing wasn't too critical for my use-case.

You could also do it on a listener of the ApplicationBooted event, which is fired after the NativeAppServiceProvider::boot method has been called and the __destruct calls happen.

I'm not sure we should introduce another approach...

@SRWieZ
Copy link
Member Author
SRWieZ commented Dec 26, 2024 •
edited
Loading

I will try that.

Another solution would be to avoid calling setTitle() if no title is provided. However, I'm unsure whether a null value or an empty string is sent.

state.activeMenuBar.on("ready", () => {
    state.activeMenuBar.tray.setTitle(label);
}

@SRWieZ
Copy link
Member Author
SRWieZ commented Dec 26, 2024

ApplicationBooted oc 8000 curs too soon, unfortunately.

Because creating the menubar is asynchronous and relatively slow (it takes about 1 second for mine, including the context menu and everything), my child process is faster at updating the menubar title (even though it connects to an API ^^).

What do you suggest ?

Maybe the MenuBarReady event? However, if I redraw my menu, I need to keep track of how many times this has occurred, as there isn't a listenOnce method in Laravel.

@simonhamp
Copy link
Member

Maybe we need a new event (MenuBarCreated) that gets fired only once, when the menubar gets created? Perhaps something like this here:

    state.activeMenuBar.on("ready", () => {
        if (! state.activeMenuBar) {
            notifyLaravel("events", {
                event: "\\Native\\Laravel\\Events\\MenuBar\\MenuBarCreated"
            });
        }
    
        state.activeMenuBar.tray.setTitle(label);
        ...
    });

@SRWieZ
Copy link
Member Author
SRWieZ commented Dec 29, 2024

Suggestion applied and tester with both PR.

@simonhamp simonhamp merged commit e700c62 into NativePHP:main Dec 29, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonhamp simonhamp simonhamp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SRWieZ @simonhamp
0