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.

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

fix favicon routes for pages that are created during or after startup #222

Merged
merged 1 commit into from Jan 5, 2023

Conversation

falkoschindler
Copy link
Contributor

Until now favicons are collected in a global dictionary. During startup a function create_favicon_routes() is called to create routes for all favicons in this dictionary. This is because we need wait until ui.run(..., favicon=...) has been called because it might define a default favicon.

But this does not work for pages that are also created during startup (but a bit later) or even dynamically caused by a user event.

from nicegui import ui

ui.label('The favicon is correct on the index page.')

def startup():
    @ui.page('/sub')
    def page():
        ui.label('There is no favicon route for this page because it was only created during startup.')

ui.on_startup(startup)

ui.run(favicon='favicon.ico')

This PR proposes to create routes directly when initializing pages. Even though we need to wait for the favicon argument in ui.run, we can already create routes and let the callback access the fallback favicon when called. This simplifies things (no need for the global dictionary anymore) and works more robustly, since pages can be created at any time and favicons will be routed.

@falkoschindler falkoschindler added the bug Something isn't working label Jan 4, 2023
@falkoschindler falkoschindler added this to the v1.0.10 milestone Jan 4, 2023
@rodja
Copy link
Member

rodja commented Jan 5, 2023

Very nice!

@rodja rodja merged commit be6a4fa into main Jan 5, 2023
@rodja rodja deleted the fix-favicons-after-startup branch January 5, 2023 03:17
@falkoschindler falkoschindler self-assigned this Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants