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

add list of list support to buttongroup widget #90

Merged
merged 7 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions example/pulseaudio_sink.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@
A simple SHC example, providing a web interface to control and supervise the default sink of the local Pulseaudio
server.
"""
from typing import cast, Iterable

import shc
import shc.web
import shc.interfaces.pulse
from shc.web.widgets import Slider, ButtonGroup, ToggleButton, icon, DisplayButton
from shc.web.widgets import AbstractButton, Slider, ButtonGroup, ToggleButton, icon, DisplayButton

interface = shc.interfaces.pulse.PulseAudioInterface()
sink_name = None
Expand All @@ -35,10 +36,10 @@
page.add_item(Slider("Volume").connect(volume.field('volume')))
page.add_item(Slider("Balance").connect(volume.field('balance'), convert=True))
page.add_item(Slider("Fade").connect(volume.field('fade'), convert=True))
page.add_item(ButtonGroup("", [
page.add_item(ButtonGroup("", cast(Iterable[AbstractButton], [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea why mypy wants this now here and not at other places, e.g. ui_showcase.py. Maybe you have an idea how to solve it without the cast. Looks kind of ugly.

Copy link
Owner

Choose a reason for hiding this comment

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

MyPy tries to guess the type of that list here, based on the first common ancestor class of the two buttons it finds. Unfortunately it finds the common ancestor WebDisplayDatapoint[bool] instead of AbstractButton. So it complains that we are passing a list of ``WebDisplayDatapoint[bool]s instead of a list of AbstractButtons`. I assume that, without the new `Union` in the parameter type hint, MyPy was smart enough to find that `List[AbstractButton]`, as expected by the parameter, is also a valid type of that list, so it used that instead of trying to guess the type.

I think, the "correct" solution would be to explicitly type-hint the list:

buttons: List[AbstractButton] = [
    ToggleButton(icon('volume mute')).connect(mute),
    DisplayButton(label=icon('power off')).connect(active),
]
page.add_item(ButtonGroup("", buttons))

However, that also looks kind of ugly, especially in the example code. So maybe, we go for a simple type-ignore with a comment, like # type: ignore # MyPy has problems with guessing the correct type of the list of buttons.
Maybe, MyPy will learn this type inference eventually and we can remove the comment. (I will check that once in a while by running MyPy with --warn-unused-ignores locally.)

ToggleButton(icon('volume mute')).connect(mute),
DisplayButton(label=icon('power off')).connect(active),
]))
])))

if __name__ == '__main__':
shc.main()
6 changes: 6 additions & 0 deletions example/ui_showcase.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ class Fruits(enum.Enum):
confirm_message="Do you want the foobar?", confirm_values=[True]).connect(foobar),
]))

# A simple ButtonGroup with nested lists of ToggleButtons for foobar
index_page.add_item(ButtonGroup("State of the foobar (grouped)", [
[ToggleButton("Foo").connect(foo), ToggleButton("Bar", color='red').connect(bar)],
[ToggleButton("Foobar", color='black').connect(foobar)],
]))

# We can also use ValueButtons to represent individual states (here in the 'outline' version)
index_page.add_item(ButtonGroup("The Foo", [
ValueButton(False, "Off", outline=True, color="black").connect(foo),
Expand Down
12 changes: 9 additions & 3 deletions shc/web/templates/widgets/buttongroup.htm
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
{% from "buttons.inc.htm" import render_button %}
<label class="shc label float-btn">{{ label }}</label>
<div class="ui buttons right floated">
{% for button in buttons %}
{{ render_button(button) }}
<div class="ui spaced horizontal list right floated">
Copy link
Owner

Choose a reason for hiding this comment

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

I like the use of ui horizontal list here. Sounds like the intended solution by SemanticUI. :)

{% for button_group in button_groups %}
<div class="item">
<div class="ui buttons">
{% for button in button_group %}
{{ render_button(button) }}
{% endfor %}
</div>
</div>
{% endfor %}
</div>
35 changes: 29 additions & 6 deletions shc/web/widgets.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,19 @@
import json
import pathlib
from os import PathLike
from typing import Any, Type, Union, Iterable, List, Generic, Tuple, TypeVar, Optional, Callable
from typing import (
Any,
cast,
Type,
Union,
Iterable,
List,
Generic,
Tuple,
TypeVar,
Optional,
Callable,
)

import markupsafe
from markupsafe import Markup
Expand Down Expand Up @@ -322,19 +334,30 @@ class ButtonGroup(WebPageItem):
`Connectable`.

:param label: The label to be shown left of the buttons
:param buttons: List of button descriptors
"""
def __init__(self, label: Union[str, Markup], buttons: Iterable["AbstractButton"]):
:param buttons: List or a List of Lists of button descriptors. A plain list of button descriptors will be
grouped all together, whereas providing multiple lists each list will be grouped together with a small gap
between each group of button descriptors.
"""
def __init__(
self,
label: Union[str, Markup],
buttons: Union[Iterable["AbstractButton"], Iterable[Iterable["AbstractButton"]]],
):
super().__init__()
self.label = label
self.buttons = buttons
if all(isinstance(item, Iterable) for item in buttons):
self.buttons: Iterable["AbstractButton"] = list(itertools.chain(*buttons))
self.button_groups = cast(Iterable[Iterable["AbstractButton"]], buttons)
else:
self.buttons = cast(Iterable["AbstractButton"], buttons)
self.button_groups = cast(Iterable[Iterable["AbstractButton"]], [buttons])

def get_connectors(self) -> Iterable[WebUIConnector]:
return self.buttons # type: ignore

async def render(self) -> str:
return await jinja_env.get_template('widgets/buttongroup.htm')\
.render_async(label=self.label, buttons=self.buttons)
.render_async(label=self.label, button_groups=self.button_groups)


class AbstractButton(metaclass=abc.ABCMeta):
Expand Down
31 changes: 28 additions & 3 deletions test/test_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import urllib.error
import http.client
from pathlib import Path
from typing import cast, Iterable

import aiohttp
from selenium import webdriver
Expand All @@ -28,6 +29,7 @@
from shc.datatypes import RangeFloat1, RGBUInt8, RangeUInt8
from shc.interfaces._helper import ReadableStatusInterface
from shc.supervisor import InterfaceStatus, ServiceStatus
from shc.web.widgets import AbstractButton
from ._helper import InterfaceThreadRunner, ExampleReadable, async_test


Expand Down Expand Up @@ -95,9 +97,30 @@ def test_page(self) -> None:
self.assertIn('Home Page', self.driver.title)
self.assertIn('Another segment', self.driver.page_source)
button = self.driver.find_element(By.XPATH, '//button[normalize-space(text()) = "Foobar"]')
self.assertIn("My button group", button.find_element(By.XPATH, '../..').text)
self.assertIn("My button group", button.find_element(By.XPATH, '../../../..').text)
button = self.driver.find_element(By.XPATH, '//button[normalize-space(text()) = "Bar"]')
self.assertIn("Another button group", button.find_element(By.XPATH, '../..').text)
self.assertIn("Another button group", button.find_element(By.XPATH, '../../../..').text)

def test_buttongroup_groups(self) -> None:
page = self.server.page('index', 'Home Page')
page.add_item(shc.web.widgets.ButtonGroup("My button group", [
[shc.web.widgets.StatelessButton(13, "Foo"),
shc.web.widgets.StatelessButton(27, "Bar")],
[shc.web.widgets.StatelessButton(142, "Gaga")],
]))

self.server_runner.start()
self.driver.get("http://localhost:42080")

# buttons in 1st group exist and are grouped
button = self.driver.find_element(By.XPATH, '//button[normalize-space(text()) = "Foo"]')
self.assertEqual("Foo\nBar", button.find_element(By.XPATH, '..').text)
button = self.driver.find_element(By.XPATH, '//button[normalize-space(text()) = "Bar"]')
self.assertEqual("Foo\nBar", button.find_element(By.XPATH, '..').text)

# button gaga in 2nd group exist and is the only member in the group
button = self.driver.find_element(By.XPATH, '//button[normalize-space(text()) = "Gaga"]')
self.assertEqual("Gaga", button.find_element(By.XPATH, '..').text)

def test_main_menu(self) -> None:
self.server.page('index', menu_entry="Home", menu_icon='home')
Expand Down Expand Up @@ -286,7 +309,9 @@ def test_buttons(self) -> None:
ExampleReadable(int, 42).connect(b4)

page = self.server.page('index')
page.add_item(shc.web.widgets.ButtonGroup("My button group", [b1, b2, b3, b4]))
page.add_item(
shc.web.widgets.ButtonGroup("My button group", cast(Iterable[AbstractButton], [b1, b2, b3, b4]))
)
mhthies marked this conversation as resolved.
Show resolved Hide resolved

with unittest.mock.patch.object(b1, '_publish') as b1_publish, \
unittest.mock.patch.object(b3, '_publish') as b3_publish, \
Expand Down