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

Improve generation of unique tab IDs #3876

Merged
merged 5 commits into from
Oct 20, 2024
Merged

Improve generation of unique tab IDs #3876

merged 5 commits into from
Oct 20, 2024

Conversation

falkoschindler
Copy link
Contributor

This PR improves the way tab IDs are generated to avoid duplicated tabs causing shared tab storage (#3872).

The ides originates from this post on StackOverflow: https://stackoverflow.com/a/36807854/3419103

I successfully tested it with this code snippet:

@ui.page('/')
async def main():
    await ui.context.client.connected()
    if 'id' not in app.storage.tab:
        app.storage.tab['id'] = str(uuid.uuid4())
    ui.label(app.storage.tab['id'])

An automated pytest would be great. But I guess we can't properly simulate duplicating tabs.

@falkoschindler falkoschindler added the bug Something isn't working label Oct 15, 2024
@falkoschindler falkoschindler added this to the 2.4 milestone Oct 15, 2024
@falkoschindler falkoschindler linked an issue Oct 15, 2024 that may be closed by this pull request
@rodja
Copy link
Member

rodja commented Oct 16, 2024

Should we not copy the tab storage? For example if we have

ui.select(['A', 'B', 'C']).bind_value(app.storage.tab, 'selection')

I would expect the duplicated tab to have the same selection. But then each should be able to edit independently. That isat least the way the browser session storage works...

@falkoschindler
Copy link
Contributor Author

@rodja You're absolutely right! I forgot this part of the equation...

@falkoschindler falkoschindler modified the milestones: 2.4, 2.5 Oct 16, 2024
@falkoschindler
Copy link
Contributor Author

Ok, now the tab storage is copied if a tab duplication is detected. Can be tested with this snippet:

import random
from nicegui import app, ui

@ui.page('/')
async def main():
    ui.add_body_html('''
        <script>
        document.addEventListener('DOMContentLoaded', function () {
            const span = document.createElement('span');
            span.innerHTML = `Tab ID: <pre>${TAB_ID}</pre>`
            if (OLD_TAB_ID) span.innerHTML += `Cloned from: <pre>${OLD_TAB_ID}</pre>`
            span.classList.add('p-2', 'rounded', 'whitespace-pre-line', 'border', 'absolute-center');
            document.body.appendChild(span);
        });
        </script>
    ''')
    await ui.context.client.connected()
    if 'number' not in app.storage.tab:
        app.storage.tab['number'] = random.randint(0, 1000000)
    ui.label(f'Number: {app.storage.tab["number"]}')

    ui.select(['A', 'B', 'C']).bind_value(app.storage.tab, 'selection')

Copy link
Member

@rodja rodja left a comment

Choose a reason for hiding this comment

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

Great!

@rodja rodja merged commit 526c66b into main Oct 20, 2024
7 checks passed
@rodja rodja deleted the tab-id branch October 20, 2024 03:46
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.

app.storage.tab is not unique for duplicate tabs
2 participants