Skip to content

Commit c5df15b

Browse files
committed
Improve HTML formatter with complete block coverage and enhanced UX
- Added comprehensive test coverage for all block types (TableBlock, CodeBlock, DividerBlock, ActionsBlock) - Improved fact list styling: removed backgrounds/borders, added max-width:200px on labels - Standardized font-size to 14px across all blocks (fact lists, tables, expandable) - Removed ActionsBlock rendering (not supported in HTML emails, matches adaptive cards) - Made expandable blocks actually expandable using HTML5 details/summary elements - Added visual arrow indicators (▶/▼) to expandable blocks, properly hiding native browser triangles - Enhanced bullet lists with proper semantic HTML (ul/li elements) and automatic pattern detection - All 36 HTML formatter tests passing
1 parent c66df63 commit c5df15b

25 files changed

Lines changed: 175 additions & 77 deletions

elementary/messages/formats/html.py

Lines changed: 118 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,9 @@
44
from typing import Sequence
55

66
from elementary.messages.blocks import (
7-
ActionBlock,
87
ActionsBlock,
98
CodeBlock,
109
DividerBlock,
11-
DropdownActionBlock,
1210
ExpandableBlock,
1311
FactBlock,
1412
FactListBlock,
@@ -24,7 +22,6 @@
2422
TableBlock,
2523
TextBlock,
2624
TextStyle,
27-
UserSelectActionBlock,
2825
WhitespaceBlock,
2926
)
3027
from elementary.messages.formats.unicode import ICON_TO_UNICODE
@@ -72,10 +69,10 @@ def format_message_block(self, block: MessageBlock | ExpandableBlock) -> str:
7269
elif isinstance(block, CodeBlock):
7370
code_html = escape(block.text)
7471
return self._wrap_section(
75-
"<pre style=\"margin:0;padding:12px;"
72+
'<pre style="margin:0;padding:12px;'
7673
"background-color:#f8fafc;border-radius:4px;"
7774
"font-family:'SFMono-Regular',Consolas,'Liberation Mono',Menlo,monospace;"
78-
"font-size:13px;line-height:1.5;white-space:pre-wrap;\">"
75+
'font-size:13px;line-height:1.5;white-space:pre-wrap;">'
7976
f"{code_html}</pre>"
8077
)
8178
elif isinstance(block, LinesBlock):
@@ -87,9 +84,12 @@ def format_message_block(self, block: MessageBlock | ExpandableBlock) -> str:
8784
elif isinstance(block, ExpandableBlock):
8885
return self._format_expandable_block(block)
8986
elif isinstance(block, DividerBlock):
90-
return '<hr style="border:none;border-top:1px solid #e5e7eb;margin:16px 0;" />'
87+
return (
88+
'<hr style="border:none;border-top:1px solid #e5e7eb;margin:16px 0;" />'
89+
)
9190
elif isinstance(block, ActionsBlock):
92-
return self._format_actions_block(block)
91+
# Not supported in HTML emails (no interactivity without JavaScript)
92+
return ""
9393
else:
9494
raise ValueError(f"Unsupported message block type: {type(block)}")
9595

@@ -123,7 +123,7 @@ def _format_inline_block(self, block: InlineBlock) -> str:
123123
elif isinstance(block, InlineCodeBlock):
124124
return (
125125
"<code style=\"font-family:'SFMono-Regular',Consolas,'Liberation Mono',Menlo,monospace;"
126-
"background-color:#eef2ff;border-radius:3px;padding:1px 4px;font-size:12px;\">"
126+
'background-color:#eef2ff;border-radius:3px;padding:1px 4px;font-size:12px;">'
127127
f"{escape(block.code)}</code>"
128128
)
129129
elif isinstance(block, MentionBlock):
@@ -141,20 +141,101 @@ def _format_line_block(self, block: LineBlock) -> str:
141141
return separator.join(inlines)
142142

143143
def _format_lines_block(self, block: LinesBlock) -> str:
144+
if not block.lines:
145+
return ""
146+
147+
# Check if this is a bullet list (all lines start with icon/bullet + space)
148+
is_bullet_list = self._is_bullet_list(block)
149+
150+
if is_bullet_list:
151+
return self._format_as_bullet_list(block)
152+
144153
lines_html = [
145154
f'<div style="margin:0;">{self._format_line_block(line_block)}</div>'
146155
for line_block in block.lines
147156
]
148-
if not lines_html:
149-
return ""
150157
return self._wrap_section("".join(lines_html))
151158

159+
def _is_bullet_list(self, block: LinesBlock) -> bool:
160+
"""Check if a LinesBlock is a bullet list pattern."""
161+
if not block.lines:
162+
return False
163+
164+
for line in block.lines:
165+
if len(line.inlines) < 3:
166+
return False
167+
# Check pattern: [optional whitespaces...] + (icon or bullet text) + space + content
168+
# Skip leading whitespaces
169+
idx = 0
170+
while idx < len(line.inlines) and isinstance(
171+
line.inlines[idx], WhitespaceBlock
172+
):
173+
idx += 1
174+
175+
if idx >= len(line.inlines):
176+
return False
177+
178+
# Next should be IconBlock or short TextBlock (bullet marker)
179+
bullet = line.inlines[idx]
180+
if isinstance(bullet, IconBlock):
181+
continue
182+
elif isinstance(bullet, TextBlock) and len(bullet.text) <= 2:
183+
continue
184+
else:
185+
return False
186+
187+
return True
188+
189+
def _format_as_bullet_list(self, block: LinesBlock) -> str:
190+
"""Format a LinesBlock as HTML <ul> list."""
191+
list_items = []
192+
for line in block.lines:
193+
# Extract bullet marker and content
194+
idx = 0
195+
# Skip leading whitespaces
196+
while idx < len(line.inlines) and isinstance(
197+
line.inlines[idx], WhitespaceBlock
198+
):
199+
idx += 1
200+
201+
# Get the bullet icon/text
202+
bullet_inline = line.inlines[idx]
203+
idx += 1
204+
205+
# Skip the space after bullet
206+
if (
207+
idx < len(line.inlines)
208+
and isinstance(line.inlines[idx], TextBlock)
209+
and line.inlines[idx].text == " "
210+
):
211+
idx += 1
212+
213+
# Rest is the content
214+
content_inlines = line.inlines[idx:]
215+
content_html = "".join(
216+
[self._format_inline_block(inline) for inline in content_inlines]
217+
)
218+
219+
# Format the bullet marker
220+
if isinstance(bullet_inline, IconBlock):
221+
bullet_html = self._format_icon(bullet_inline.icon)
222+
list_items.append(
223+
f'<li style="margin:0 0 4px;list-style:none;">'
224+
f'<span style="margin-right:6px;">{bullet_html}</span>{content_html}</li>'
225+
)
226+
else:
227+
# Text bullet - use native list styling
228+
list_items.append(f'<li style="margin:0 0 4px;">{content_html}</li>')
229+
230+
ul_style = "margin:0;padding-left:24px;list-style-position:outside;"
231+
return self._wrap_section(f'<ul style="{ul_style}">{"".join(list_items)}</ul>')
232+
152233
def _format_fact_list_block(self, block: FactListBlock) -> str:
153234
if not block.facts:
154235
return ""
155236
rows = [self._format_fact_row(fact) for fact in block.facts]
156237
table_html = (
157-
'<table style="width:100%;border-collapse:separate;border-spacing:0 6px;">'
238+
'<table style="width:100%;border-collapse:collapse;">'
158239
+ "".join(rows)
159240
+ "</table>"
160241
)
@@ -164,15 +245,11 @@ def _format_fact_row(self, fact: FactBlock) -> str:
164245
title_html = self._format_line_block(fact.title)
165246
value_html = self._format_line_block(fact.value)
166247
title_style = (
167-
"padding:4px 12px;font-weight:600;color:#111827;"
168-
"background-color:#f3f4f6;border-radius:4px 0 0 4px;"
169-
"white-space:nowrap;"
248+
"padding:4px 12px;font-weight:600;font-size:14px;color:#111827;"
249+
"max-width:200px;white-space:nowrap;"
170250
)
171251
value_weight = "700" if fact.primary else "400"
172-
value_style = (
173-
"padding:4px 12px;border:1px solid #f3f4f6;border-left:none;"
174-
"border-radius:0 4px 4px 0;font-weight:{weight};"
175-
).format(weight=value_weight)
252+
value_style = f"padding:4px 12px;font-weight:{value_weight};font-size:14px;"
176253
return (
177254
"<tr>"
178255
f'<td style="{title_style}">{title_html}</td>'
@@ -185,14 +262,14 @@ def _format_table_block(self, block: TableBlock) -> str:
185262
if block.headers:
186263
header_cells = "".join(
187264
f'<th style="text-align:left;padding:8px;border-bottom:1px solid #e5e7eb;'
188-
f'font-weight:600;background-color:#f8fafc;">{escape(header)}</th>'
265+
f'font-weight:600;font-size:14px;background-color:#f8fafc;">{escape(header)}</th>'
189266
for header in block.headers
190267
)
191268
header_html = f"<thead><tr>{header_cells}</tr></thead>"
192269
body_rows = [
193270
"<tr>"
194271
+ "".join(
195-
f'<td style="padding:8px;border-bottom:1px solid #f3f4f6;vertical-align:top;">'
272+
f'<td style="padding:8px;border-bottom:1px solid #f3f4f6;vertical-align:top;font-size:14px;">'
196273
f"{escape(self._coerce_table_cell(cell))}</td>"
197274
for cell in row
198275
)
@@ -209,46 +286,33 @@ def _format_table_block(self, block: TableBlock) -> str:
209286
def _format_expandable_block(self, block: ExpandableBlock) -> str:
210287
body_html = self.format_message_blocks(block.body)
211288
title_html = escape(block.title)
212-
container_style = (
213-
"border:1px solid #e5e7eb;border-radius:6px;margin:16px 0;overflow:hidden;"
289+
container_style = "border:1px solid #e5e7eb;border-radius:6px;margin:16px 0;"
290+
# Hide native disclosure triangle with list-style and webkit-details-marker
291+
summary_style = (
292+
"padding:12px 16px;font-weight:600;background-color:#f8fafc;"
293+
"cursor:pointer;font-size:14px;user-select:none;-webkit-user-select:none;"
294+
"list-style:none;"
214295
)
215-
title_style = (
216-
"margin:0;padding:12px 16px;font-weight:600;background-color:#f8fafc;"
296+
# Show appropriate arrow based on expanded state
297+
arrow = "▼" if block.expanded else "▶"
298+
arrow_style = "margin-right:8px;color:#6b7280;font-size:10px;"
299+
body_style = "padding:12px 16px;border-top:1px solid #e5e7eb;"
300+
open_attr = " open" if block.expanded else ""
301+
# Need inline style to hide webkit disclosure marker
302+
summary_with_marker_hidden = (
303+
f'<summary style="{summary_style}">'
304+
f'<span style="{arrow_style}">{arrow}</span>'
305+
f"{title_html}"
306+
"</summary>"
307+
"<style>summary::-webkit-details-marker{display:none;}</style>"
217308
)
218-
body_style = "padding:12px 16px;"
219309
return (
220-
f'<div style="{container_style}">'
221-
f'<div style="{title_style}">{title_html}</div>'
310+
f'<details style="{container_style}"{open_attr}>'
311+
f"{summary_with_marker_hidden}"
222312
f'<div style="{body_style}">{body_html}</div>'
223-
"</div>"
313+
"</details>"
224314
)
225315

226-
def _format_actions_block(self, block: ActionsBlock) -> str:
227-
if not block.actions:
228-
return ""
229-
rendered_actions = [self._format_action_item(action) for action in block.actions]
230-
actions_html = "".join(rendered_actions)
231-
return self._wrap_section(
232-
f'<div style="display:flex;flex-wrap:wrap;gap:8px;">{actions_html}</div>'
233-
)
234-
235-
def _format_action_item(self, block: ActionBlock) -> str:
236-
if isinstance(block, DropdownActionBlock):
237-
options = ", ".join(escape(option.text) for option in block.options)
238-
placeholder = escape(block.placeholder or "Select an option")
239-
return (
240-
'<div style="padding:8px 12px;border:1px solid #d1d5db;border-radius:4px;'
241-
f'background-color:#f9fafb;">{placeholder}: {options}</div>'
242-
)
243-
elif isinstance(block, UserSelectActionBlock):
244-
placeholder = escape(block.placeholder or "Assign user")
245-
return (
246-
'<div style="padding:8px 12px;border:1px solid #d1d5db;border-radius:4px;'
247-
f'background-color:#f9fafb;">{placeholder}</div>'
248-
)
249-
else:
250-
raise ValueError(f"Unsupported action block type: {type(block)}")
251-
252316
def _coerce_table_cell(self, cell: object) -> str:
253317
if cell is None:
254318
return ""

tests/unit/messages/formats/base_test_format.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,11 @@
66

77
from elementary.messages.block_builders import BulletListBlock
88
from elementary.messages.blocks import (
9+
ActionsBlock,
910
CodeBlock,
1011
DividerBlock,
12+
DropdownActionBlock,
13+
DropdownOptionBlock,
1114
ExpandableBlock,
1215
FactBlock,
1316
FactListBlock,
@@ -22,6 +25,7 @@
2225
TableBlock,
2326
TextBlock,
2427
TextStyle,
28+
UserSelectActionBlock,
2529
WhitespaceBlock,
2630
)
2731
from elementary.messages.message_body import Color, MessageBody
@@ -233,6 +237,8 @@ def test_format_message_body_all_blocks(self, color, expected_file):
233237
)
234238
]
235239
),
240+
CodeBlock(text="def hello_world():\n print('Hello, World!')"),
241+
DividerBlock(),
236242
BulletListBlock(
237243
icon="-",
238244
lines=[
@@ -256,6 +262,13 @@ def test_format_message_body_all_blocks(self, color, expected_file):
256262
),
257263
]
258264
),
265+
TableBlock(
266+
headers=["Column 1", "Column 2", "Column 3"],
267+
rows=[
268+
["Row 1 Col 1", "Row 1 Col 2", "Row 1 Col 3"],
269+
["Row 2 Col 1", "Row 2 Col 2", "Row 2 Col 3"],
270+
],
271+
),
259272
ExpandableBlock(
260273
title="Show Details",
261274
body=[
@@ -282,6 +295,23 @@ def test_format_message_body_all_blocks(self, color, expected_file):
282295
],
283296
expanded=False,
284297
),
298+
ActionsBlock(
299+
actions=[
300+
DropdownActionBlock(
301+
action_id="priority_dropdown",
302+
placeholder="Select priority",
303+
options=[
304+
DropdownOptionBlock(text="High", value="high"),
305+
DropdownOptionBlock(text="Medium", value="medium"),
306+
DropdownOptionBlock(text="Low", value="low"),
307+
],
308+
),
309+
UserSelectActionBlock(
310+
action_id="assign_user",
311+
placeholder="Assign to user",
312+
),
313+
]
314+
),
285315
LinesBlock(
286316
lines=[
287317
LineBlock(

0 commit comments

Comments
 (0)