-
-
Notifications
You must be signed in to change notification settings - Fork 640
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
Load dependencies in parallel when updating UI elements #4235
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small example for testing:
from nicegui import ui
from nicegui.element import Element
class Circle(Element, component='circle.js'):
def __init__(self, x=0, y=0, r=10, fill='red', **kwargs):
super().__init__(**kwargs)
self._props['x'] = x
self._props['y'] = y
self._props['r'] = r
self._props['fill'] = fill
@ui.page('/')
def test_page() -> None:
rows = 20
cols = 20
x_offset = 20
y_offset = 20
x_spacing = 40
y_spacing = 40
with ui.column():
ui.label('Circles').classes('text-xl mb-2')
circle_container = ui.element('svg').classes('w-[800px] h-[800px] mb-4')
circles: list[Element] = []
def create_circles() -> None:
with circle_container:
for i in range(rows):
for j in range(cols):
element = Circle(x=x_offset + i * x_spacing, y=y_offset + j * y_spacing, r=5, fill='red')
circles.append(element)
def remove_circles() -> None:
for circle in circles:
circle.delete()
circles.clear()
async def change_circle_colors() -> None:
for circle in circles:
circle.props('fill=blue')
async def change_circle_sizes() -> None:
for circle in circles:
circle.props('r=10')
with ui.row():
(ui.button('Create Grid', on_click=create_circles)
.props('flat dense color="primary"'))
(ui.button('Remove Grid', on_click=remove_circles)
.props('flat dense color="primary"'))
(ui.button('Change Colors', on_click=change_circle_colors)
.props('flat dense color="primary"'))
(ui.button('Change Sizes', on_click=change_circle_sizes)
.props('flat dense color="primary"'))
ui.run(port=8040)
circle.js:
export default {
template: `
<circle :cx="x" :cy="y" :r="r" fill="none" :stroke="fill" stroke-width="1" />
`,
props: {
r: Number,
fill: String,
x: Number,
y: Number,
},
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NiklasNeugebauer Great idea to load all dependencies at once!
I just wonder why we need to await nextTick
. We didn't do that before. 🤔
Minimum reproduction: ui.button('Toggle', on_click=lambda: [element.classes(toggle='border') for element in elements])
elements = [ui.markdown(f'{i}') for i in range(200)] |
I think the dependency loading makes no significant difference since most are served from the Nicegui Server anyway but it felt better than doing them one after the other (maybe there is some optimization or multi-processing on the server after all). The core issue was the ticks. Currently, (as I understand it) each change to the |
Looking at the documentation again my explanation seems to be incorrect. |
Yes, the microtask for the Vue update can only run after the function so this cannot make any difference. It seems that the loading functions were really the core problem then. I removed the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! Less diff, same effect 🚀
I noticed that adding or changing a large number of elements, especially inside an SVG tag, takes very long.
This changes the update handler to first load all dependencies in parallel and then update the elements.