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.

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

Correct the creation of menubars in the web backend. #2195

Merged
merged 4 commits into from
Nov 7, 2023
Merged

Correct the creation of menubars in the web backend. #2195

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
daf3cf3
Correct the creation of menubars in the web backend.
freakboy3742 Nov 5, 2023
14b860e
Add changenote for about dialog fix.
freakboy3742 Nov 5, 2023
f8fa58c
Correct handling of DOM click events on menu items.
freakboy3742 Nov 6, 2023
cf9eee3
Use a consistent pattern on backends that use the same menu activatio…
freakboy3742 Nov 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion changes/2103.docs.rst
View file
Open in desktop

This file was deleted.

1 change: 1 addition & 0 deletions changes/2194.bugfix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The web backend no longer generates a duplicate titlebar.
1 change: 1 addition & 0 deletions changes/2195.bugfix.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
An issue with the display of the About dialog on the web backend was corrected.
19 changes: 15 additions & 4 deletions web/src/toga_web/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def create(self):
self.interface.commands.add(
# ---- Help menu ----------------------------------
toga.Command(
lambda _: self.interface.about(),
Copy link
Member

Choose a reason for hiding this comment

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

I don't follow this change. Previously it was taking one argument (the Command), and ignoring it. The other backends do the same thing. Now it's taking two arguments (event, widget)? How does that fit the protocol?

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh... I made the change to get it to work, but didn't mentally catch that the resulting method was no longer following the same protocol. I'll dig deeper into what is going on.

Copy link
Member Author

Choose a reason for hiding this comment

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

🤦

The on_click handler needs to be a DOM handler; it was being assigned the raw Toga handler. It worked previously because the first argument to the toga handler was eaten as part of the binding process, so the action would be invoked with the DOM event as the "widget". With the removal of the redundant argument, it was no longer being consumed.

Fixed by making the on_click handler an actual DOM event handler, which proxies to activate the action.

The same pattern is used by Winforms and GTK, so I've modified the code to use a consistent implementation approach. Android and Cocoa use a different approach - there's a central "menu item selected" event which triggers a lookup and activation of the appropriate action.

self._menu_about,
"About " + formal_name,
group=toga.Group.HELP,
),
Expand All @@ -34,7 +34,9 @@ def create(self):
),
)

# Create the menus.
# Create the menus. This is done before main window content to ensure
# the <header> for the menubar is inserted before the <main> for the
# main window.
self.create_menus()

# Call user code to populate the main window
Expand Down Expand Up @@ -103,8 +105,10 @@ def create_menus(self):
else:
help_menu_container.appendChild(submenu)

menubar_id = f"{self.interface.app_id}-header"
self.menubar = create_element(
"header",
id=menubar_id,
classes=["toga"],
children=[
create_element(
Expand All @@ -124,8 +128,15 @@ def create_menus(self):
],
)

# Menubar exists at the app level.
self.native.appendChild(self.menubar)
# If there's an existing menubar, replace it.
old_menubar = js.document.getElementById(menubar_id)
if old_menubar:
old_menubar.replaceWith(self.menubar)
else:
self.native.append(self.menubar)

def _menu_about(self, event, widget, **kwargs):
self.interface.about()

def main_loop(self):
self.create()
Expand Down
Loading