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

add list of list support to buttongroup widget #90

Merged
merged 7 commits into from
Oct 30, 2024
Merged

add list of list support to buttongroup widget #90

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d56cfce
add list of list support to buttongroup widget
jo47011 Oct 19, 2024
cf94d0e
remove package-lock.json
jo47011 Oct 19, 2024
6a93233
fix: flake issues
jo47011 Oct 19, 2024
97fd8b1
fix: tests
jo47011 Oct 19, 2024
920f207
add: parameter description details
jo47011 Oct 20, 2024
531087b
add: button group test
jo47011 Oct 20, 2024
f017ccd
fix: flake issue
jo47011 Oct 28, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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>
31 changes: 26 additions & 5 deletions shc/web/widgets.py
View file
Open in desktop
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,28 @@ class ButtonGroup(WebPageItem):
`Connectable`.

:param label: The label to be shown left of the buttons
:param buttons: List of button descriptors
:param buttons: List or a List if List of button descriptors
Copy link
Owner

Choose a reason for hiding this comment

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

One afterthought: Can you please add a more detailed description on how to use the parameter, i.e. what effect the single list of button descriptors and the list of lists of button descriptors will have? You can either write it in the parameter description or as a separate paragraph above.

"""
def __init__(self, label: Union[str, Markup], buttons: Iterable["AbstractButton"]):
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
10 changes: 7 additions & 3 deletions test/test_web.py
View file
Open in desktop
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,9 @@ 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_main_menu(self) -> None:
self.server.page('index', menu_entry="Home", menu_icon='home')
Expand Down Expand Up @@ -286,7 +288,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
Loading