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

Merged
simonhamp merged 5 commits intoNativePHP:mainfrom
SRWieZ:fix-menubar-not-ready
Dec 29, 2024
Merged

Fix menubar not ready#453
simonhamp merged 5 commits intoNativePHP:mainfrom
SRWieZ:fix-menubar-not-ready

Conversation

@SRWieZ
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
8000 Member Author
SRWieZ commented Dec 26, 2024

ApplicationBooted occurs 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
Copy Markdown
Member
simonhamp commented Dec 29, 2024

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
Copy Markdown
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
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