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

feat: Display image in a graphics_card #1817 #2373

Merged
merged 4 commits into from
Aug 7, 2024
Merged

feat: Display image in a graphics_card #1817 #2373

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
104a312
feat: Display image in a graphics_card #1817
dbranley Jul 28, 2024
9ac54c1
feat: Changes from review comments, remove parent div, revert graphic…
dbranley Aug 6, 2024
3c79f10
chore: Format, rename props.
mturoci Aug 7, 2024
7d4e51a
docs: Reword.
mturoci Aug 7, 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
30 changes: 30 additions & 0 deletions py/h2o_lightwave/h2o_lightwave/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8984,12 +8984,18 @@ def __init__(
scene: Optional[PackedData] = None,
width: Optional[str] = None,
height: Optional[str] = None,
path: Optional[str] = None,
image: Optional[str] = None,
type: Optional[str] = None,
commands: Optional[List[Command]] = None,
):
_guard_scalar('GraphicsCard.box', box, (str,), False, False, False)
_guard_scalar('GraphicsCard.view_box', view_box, (str,), False, False, False)
_guard_scalar('GraphicsCard.width', width, (str,), False, True, False)
_guard_scalar('GraphicsCard.height', height, (str,), False, True, False)
_guard_scalar('GraphicsCard.path', path, (str,), False, True, False)
_guard_scalar('GraphicsCard.image', image, (str,), False, True, False)
_guard_scalar('GraphicsCard.type', type, (str,), False, True, False)
_guard_vector('GraphicsCard.commands', commands, (Command,), False, True, False)
self.box = box
"""A string indicating how to place this component on the page."""
Expand All @@ -9003,6 +9009,12 @@ def __init__(
"""The displayed width of the rectangular viewport. (Not the width of its coordinate system.)"""
self.height = height
"""The displayed height of the rectangular viewport. (Not the height of its coordinate system.)"""
self.path = path
"""The path or URL or data URL of the background image, e.g. `/foo.png` or `http://example.com/foo.png` or `data:image/png;base64,???`."""
self.image = image
"""Background image data, base64-encoded."""
self.type = type
"""The background image MIME subtype. One of `apng`, `bmp`, `gif`, `x-icon`, `jpeg`, `png`, `webp`. Required only if `image` is set."""
self.commands = commands
"""Contextual menu commands for this component."""

Expand All @@ -9012,6 +9024,9 @@ def dump(self) -> Dict:
_guard_scalar('GraphicsCard.view_box', self.view_box, (str,), False, False, False)
_guard_scalar('GraphicsCard.width', self.width, (str,), False, True, False)
_guard_scalar('GraphicsCard.height', self.height, (str,), False, True, False)
_guard_scalar('GraphicsCard.path', self.path, (str,), False, True, False)
_guard_scalar('GraphicsCard.image', self.image, (str,), False, True, False)
_guard_scalar('GraphicsCard.type', self.type, (str,), False, True, False)
_guard_vector('GraphicsCard.commands', self.commands, (Command,), False, True, False)
return _dump(
view='graphics',
Expand All @@ -9021,6 +9036,9 @@ def dump(self) -> Dict:
scene=self.scene,
width=self.width,
height=self.height,
path=self.path,
image=self.image,
type=self.type,
commands=None if self.commands is None else [__e.dump() for __e in self.commands],
)

Expand All @@ -9037,6 +9055,12 @@ def load(__d: Dict) -> 'GraphicsCard':
_guard_scalar('GraphicsCard.width', __d_width, (str,), False, True, False)
__d_height: Any = __d.get('height')
_guard_scalar('GraphicsCard.height', __d_height, (str,), False, True, False)
__d_path: Any = __d.get('path')
_guard_scalar('GraphicsCard.path', __d_path, (str,), False, True, False)
__d_image: Any = __d.get('image')
_guard_scalar('GraphicsCard.image', __d_image, (str,), False, True, False)
__d_type: Any = __d.get('type')
_guard_scalar('GraphicsCard.type', __d_type, (str,), False, True, False)
__d_commands: Any = __d.get('commands')
_guard_vector('GraphicsCard.commands', __d_commands, (dict,), False, True, False)
box: str = __d_box
Expand All @@ -9045,6 +9069,9 @@ def load(__d: Dict) -> 'GraphicsCard':
scene: Optional[PackedData] = __d_scene
width: Optional[str] = __d_width
height: Optional[str] = __d_height
path: Optional[str] = __d_path
image: Optional[str] = __d_image
type: Optional[str] = __d_type
commands: Optional[List[Command]] = None if __d_commands is None else [Command.load(__e) for __e in __d_commands]
return GraphicsCard(
box,
Expand All @@ -9053,6 +9080,9 @@ def load(__d: Dict) -> 'GraphicsCard':
scene,
width,
height,
path,
image,
type,
commands,
)

Expand Down
9 changes: 9 additions & 0 deletions py/h2o_lightwave/h2o_lightwave/ui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3121,6 +3121,9 @@ def graphics_card(
scene: Optional[PackedData] = None,
width: Optional[str] = None,
height: Optional[str] = None,
path: Optional[str] = None,
image: Optional[str] = None,
type: Optional[str] = None,
commands: Optional[List[Command]] = None,
) -> GraphicsCard:
"""Create a card for displaying vector graphics.
Expand All @@ -3132,6 +3135,9 @@ def graphics_card(
scene: Foreground layer for rendering dynamic SVG elements.
width: The displayed width of the rectangular viewport. (Not the width of its coordinate system.)
height: The displayed height of the rectangular viewport. (Not the height of its coordinate system.)
path: The path or URL or data URL of the background image, e.g. `/foo.png` or `http://example.com/foo.png` or `data:image/png;base64,???`.
image: Background image data, base64-encoded.
type: The background image MIME subtype. One of `apng`, `bmp`, `gif`, `x-icon`, `jpeg`, `png`, `webp`. Required only if `image` is set.
commands: Contextual menu commands for this component.
Returns:
A `h2o_wave.types.GraphicsCard` instance.
Expand All @@ -3143,6 +3149,9 @@ def graphics_card(
scene,
width,
height,
path,
image,
type,
commands,
)

Expand Down
30 changes: 30 additions & 0 deletions py/h2o_wave/h2o_wave/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8984,12 +8984,18 @@ def __init__(
scene: Optional[PackedData] = None,
width: Optional[str] = None,
height: Optional[str] = None,
path: Optional[str] = None,
image: Optional[str] = None,
type: Optional[str] = None,
commands: Optional[List[Command]] = None,
):
_guard_scalar('GraphicsCard.box', box, (str,), False, False, False)
_guard_scalar('GraphicsCard.view_box', view_box, (str,), False, False, False)
_guard_scalar('GraphicsCard.width', width, (str,), False, True, False)
_guard_scalar('GraphicsCard.height', height, (str,), False, True, False)
_guard_scalar('GraphicsCard.path', path, (str,), False, True, False)
_guard_scalar('GraphicsCard.image', image, (str,), False, True, False)
_guard_scalar('GraphicsCard.type', type, (str,), False, True, False)
_guard_vector('GraphicsCard.commands', commands, (Command,), False, True, False)
self.box = box
"""A string indicating how to place this component on the page."""
Expand All @@ -9003,6 +9009,12 @@ def __init__(
"""The displayed width of the rectangular viewport. (Not the width of its coordinate system.)"""
self.height = height
"""The displayed height of the rectangular viewport. (Not the height of its coordinate system.)"""
self.path = path
"""The path or URL or data URL of the background image, e.g. `/foo.png` or `http://example.com/foo.png` or `data:image/png;base64,???`."""
self.image = image
"""Background image data, base64-encoded."""
self.type = type
"""The background image MIME subtype. One of `apng`, `bmp`, `gif`, `x-icon`, `jpeg`, `png`, `webp`. Required only if `image` is set."""
self.commands = commands
"""Contextual menu commands for this component."""

Expand All @@ -9012,6 +9024,9 @@ def dump(self) -> Dict:
_guard_scalar('GraphicsCard.view_box', self.view_box, (str,), False, False, False)
_guard_scalar('GraphicsCard.width', self.width, (str,), False, True, False)
_guard_scalar('GraphicsCard.height', self.height, (str,), False, True, False)
_guard_scalar('GraphicsCard.path', self.path, (str,), False, True, False)
_guard_scalar('GraphicsCard.image', self.image, (str,), False, True, False)
_guard_scalar('GraphicsCard.type', self.type, (str,), False, True, False)
_guard_vector('GraphicsCard.commands', self.commands, (Command,), False, True, False)
return _dump(
view='graphics',
Expand All @@ -9021,6 +9036,9 @@ def dump(self) -> Dict:
scene=self.scene,
width=self.width,
height=self.height,
path=self.path,
image=self.image,
type=self.type,
commands=None if self.commands is None else [__e.dump() for __e in self.commands],
)

Expand All @@ -9037,6 +9055,12 @@ def load(__d: Dict) -> 'GraphicsCard':
_guard_scalar('GraphicsCard.width', __d_width, (str,), False, True, False)
__d_height: Any = __d.get('height')
_guard_scalar('GraphicsCard.height', __d_height, (str,), False, True, False)
__d_path: Any = __d.get('path')
_guard_scalar('GraphicsCard.path', __d_path, (str,), False, True, False)
__d_image: Any = __d.get('image')
_guard_scalar('GraphicsCard.image', __d_image, (str,), False, True, False)
__d_type: Any = __d.get('type')
_guard_scalar('GraphicsCard.type', __d_type, (str,), False, True, False)
__d_commands: Any = __d.get('commands')
_guard_vector('GraphicsCard.commands', __d_commands, (dict,), False, True, False)
box: str = __d_box
Expand All @@ -9045,6 +9069,9 @@ def load(__d: Dict) -> 'GraphicsCard':
scene: Optional[PackedData] = __d_scene
width: Optional[str] = __d_width
height: Optional[str] = __d_height
path: Optional[str] = __d_path
image: Optional[str] = __d_image
type: Optional[str] = __d_type
commands: Optional[List[Command]] = None if __d_commands is None else [Command.load(__e) for __e in __d_commands]
return GraphicsCard(
box,
Expand All @@ -9053,6 +9080,9 @@ def load(__d: Dict) -> 'GraphicsCard':
scene,
width,
height,
path,
image,
type,
commands,
)

Expand Down
9 changes: 9 additions & 0 deletions py/h2o_wave/h2o_wave/ui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3121,6 +3121,9 @@ def graphics_card(
scene: Optional[PackedData] = None,
width: Optional[str] = None,
height: Optional[str] = None,
path: Optional[str] = None,
image: Optional[str] = None,
type: Optional[str] = None,
commands: Optional[List[Command]] = None,
) -> GraphicsCard:
"""Create a card for displaying vector graphics.
Expand All @@ -3132,6 +3135,9 @@ def graphics_card(
scene: Foreground layer for rendering dynamic SVG elements.
width: The displayed width of the rectangular viewport. (Not the width of its coordinate system.)
height: The displayed height of the rectangular viewport. (Not the height of its coordinate system.)
path: The path or URL or data URL of the background image, e.g. `/foo.png` or `http://example.com/foo.png` or `data:image/png;base64,???`.
image: Background image data, base64-encoded.
type: The background image MIME subtype. One of `apng`, `bmp`, `gif`, `x-icon`, `jpeg`, `png`, `webp`. Required only if `image` is set.
commands: Contextual menu commands for this component.
Returns:
A `h2o_wave.types.GraphicsCard` instance.
Expand All @@ -3143,6 +3149,9 @@ def graphics_card(
scene,
width,
height,
path,
image,
type,
commands,
)

Expand Down
14 changes: 14 additions & 0 deletions r/R/ui.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3639,6 +3639,11 @@ ui_frame_card <- function(
#' (Not the width of its coordinate system.)
#' @param height The displayed height of the rectangular viewport.
#' (Not the height of its coordinate system.)
#' @param path The path or URL or data URL of the background image,
#' e.g. `/foo.png` or `http://example.com/foo.png` or `data:image/png;base64,???`.
#' @param image Background image data, base64-encoded.
#' @param type The background image MIME subtype. One of `apng`, `bmp`, `gif`, `x-icon`, `jpeg`, `png`, `webp`.
#' Required only if `image` is set.
#' @param commands Contextual menu commands for this component.
#' @return A GraphicsCard instance.
#' @export
Expand All @@ -3649,13 +3654,19 @@ ui_graphics_card <- function(
scene = NULL,
width = NULL,
height = NULL,
path = NULL,
image = NULL,
type = NULL,
commands = NULL) {
.guard_scalar("box", "character", box)
.guard_scalar("view_box", "character", view_box)
# TODO Validate stage: Recs
# TODO Validate scene: Data
.guard_scalar("width", "character", width)
.guard_scalar("height", "character", height)
.guard_scalar("path", "character", path)
.guard_scalar("image", "character", image)
.guard_scalar("type", "character", type)
.guard_vector("commands", "WaveCommand", commands)
.o <- list(
box=box,
Expand All @@ -3664,6 +3675,9 @@ ui_graphics_card <- function(
scene=scene,
width=width,
height=height,
path=path,
image=image,
type=type,
commands=commands,
view='graphics')
class(.o) <- append(class(.o), c(.wave_obj, "WaveGraphicsCard"))
Expand Down
5 changes: 4 additions & 1 deletion tools/intellij-plugin/src/main/resources/templates/wave-components.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1473,13 +1473,16 @@
<option name="Python" value="true"/>
</context>
</template>
<template name="w_full_graphics_card" value="ui.graphics_card(box='$box$',view_box='$view_box$',stage=$stage$,scene=$scene$,width='$width$',height='$height$',commands=[&#10; $commands$ &#10;])$END$" description="Create Wave GraphicsCard with full attributes." toReformat="true" toShortenFQNames="true">
<template name="w_full_graphics_card" value="ui.graphics_card(box='$box$',view_box='$view_box$',stage=$stage$,scene=$scene$,width='$width$',height='$height$',path='$path$',image='$image$',type='$type$',commands=[&#10; $commands$ &#10;])$END$" description="Create Wave GraphicsCard with full attributes." toReformat="true" toShortenFQNames="true">
<variable name="box" expression="" defaultValue="" alwaysStopAt="true"/>
<variable name="view_box" expression="" defaultValue="" alwaysStopAt="true"/>
<variable name="stage" expression="" defaultValue="&quot;None&quot;" alwaysStopAt="true"/>
<variable name="scene" expression="" defaultValue="&quot;None&quot;" alwaysStopAt="true"/>
<variable name="width" expression="" defaultValue="" alwaysStopAt="true"/>
<variable name="height" expression="" defaultValue="" alwaysStopAt="true"/>
<variable name="path" expression="" defaultValue="" alwaysStopAt="true"/>
<variable name="image" expression="" defaultValue="" alwaysStopAt="true"/>
<variable name="type" expression="" defaultValue="" alwaysStopAt="true"/>
<variable name="commands" expression="" defaultValue="" alwaysStopAt="true"/>
<context>
<option name="Python" value="true"/>
Expand Down
2 changes: 1 addition & 1 deletion tools/vscode-extension/component-snippets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1234,7 +1234,7 @@
"Wave Full GraphicsCard": {
"prefix": "w_full_graphics_card",
"body": [
"ui.graphics_card(box='$1', view_box='$2', stage=${3:None}, scene=${4:None}, width='$5', height='$6', commands=[\n\t\t$7\t\t\n])$0"
"ui.graphics_card(box='$1', view_box='$2', stage=${3:None}, scene=${4:None}, width='$5', height='$6', path='$7', image='$8', type='$9', commands=[\n\t\t$10\t\t\n])$0"
],
"description": "Create a full Wave GraphicsCard."
},
Expand Down
11 changes: 11 additions & 0 deletions ui/src/graphics.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ const
name,
state: {},
changed: T.box(false)
},
graphicsBackgroundImageProps: T.Model<any> = {
name,
state: { path: 'foobar.png', image : 'dummy', type : 'png'},
changed: T.box(false),
}

describe('Graphics.tsx', () => {
Expand All @@ -32,4 +37,10 @@ describe('Graphics.tsx', () => {
expect(queryByTestId(name)).toBeInTheDocument()
})

it('Renders with background image', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's replace this unit test with an entry in widgets section. The best place would probably be Widgets > Plots > Graphics or Widgets > Plots > AI (whatever makes more sense to you) and include 2 variations, a simple graphic and a simple graphic with a background image. That way, our documentation will be better and this functionality will be picked up by our visual regression testing suite.

Feel free to reach out in case you need help with any of the above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @mturoci, question for you about this. Would it be better to add this to Guide > Graphics instead since that page already explains the ui.graphics_card? I could add a new section at the bottom to explain the new background image feature. That seems like a more logical place than Widgets > Plots, but maybe I'm overlooking something?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Guide > Graphics isn't picked up by visual regression testing suite + is more of a docs on how to work with graphics rather than how to display a widget and all its variations. The widget page should definitely have a link to the graphics guide though.

Copy link
Contributor Author

@dbranley dbranley Aug 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@mturoci, As requested, I removed the new test. For the documentation, I decided to add the new Graphics section at Widgets > Content > Graphics, just after the section for Image. The graphics stuff didn't seem to fit with the other information under Widgets > Plots, but it seemed to go well with the information under Widgets > Content. Let me know if you disagree and I can move it to wherever you prefer. I built the documentation and here's a short video showing the new page:

issue1817_doc-widget.mov

const { queryByTestId } = render(<View {...graphicsBackgroundImageProps} />)
expect(queryByTestId(name)).toBeInTheDocument()
expect(queryByTestId(name)?.parentElement?.style.backgroundImage).toEqual('url(foobar.png)')
})

})
36 changes: 29 additions & 7 deletions ui/src/graphics.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,19 @@ interface State {
* (Not the height of its coordinate system.)
*/
height?: S
/**
* The path or URL or data URL of the background image,
* e.g. `/foo.png` or `http://example.com/foo.png` or `data:image/png;base64,???`.
*/
path?: S
/** Background image data, base64-encoded. */
image?: S
/**
* The background image MIME subtype. One of `apng`, `bmp`, `gif`, `x-icon`, `jpeg`, `png`, `webp`.
* Required only if `image` is set.
*/
type?: S

}

const
Expand Down Expand Up @@ -169,18 +182,27 @@ export const
},
render = () => {
const
{ view_box, width, height, stage, scene } = state,
{ view_box, width, height, stage, scene, type, image, path } = state,
stageEls = stage ? unpack<Recs>(stage).map(renderEl) : [],
sceneEls = scene ? unpack<El[]>(scene).map(({ d, o }, i) => renderEl({
...(d ? JSON.parse(d) : {}),
...(o ? JSON.parse(o) : {}),
}, i)) : []
}, i)) : [],
backgroundImageSrc = path
? 'url('+path+')'
: image ? `url(data:image/${type};base64,${image})`
: undefined

return (
<div data-test={name} className={css.card}>
<svg viewBox={view_box} width={width} height={height}>
<g>{stageEls}</g>
<g>{sceneEls}</g>
</svg>
<div style={{ backgroundImage: `${backgroundImageSrc ? backgroundImageSrc : undefined}`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this wrapper necessary? Wouldn't just adding style to an existing wrapper be enough?

Copy link
Contributor Author

@dbranley dbranley Aug 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@mturoci, that was my original idea too. But the problem is that the existing div is rendered with an absolute positioning offset of 15px around all sides. So when I put the background-image on that div, it does not cover the entire card. Here's a screen show showing what that looks like:

issue1817_backgroundBorder

I tried various things to adjust the styling of the div -- I tried setting margin and padding, but nothing looked right. Plus I was concerned about backwards compatibility issues when changing the styling of the main div with the graphics. As a result, I decided to add a new parent div. If you can think of another approach I'm overlooking, let me know!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. I think the offset is fine though because even though the image in your current implementation would take up the whole card, the outer 15px wouldn't be reachable by graphics render, if I understood it correctly, which would be a problem especially when a user would want to highlight a certain part of the image with a rectangle for example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @mturoci. So just to be clear, you are saying to remove the new parent div and instead put the background image on the existing div, which is defined with the 15px offset - right? This means the background image will not cover the entire card, as you can see in this screen shot. Please confirm that I'm understanding correctly. Thanks!!

issue1817_withBorders

Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mturoci I made the changes as suggested. Removed the parent div and added the background-image stuff to the existing div. Here's a screen shot of the custom example I've been using:

issue1817_afterChanges

backgroundSize: `${backgroundImageSrc ? 'cover' : undefined}`,
}}>
<div data-test={name} className={css.card}>
<svg viewBox={view_box} width={width} height={height}>
<g>{stageEls}</g>
<g>{sceneEls}</g>
</svg>
</div>
</div>
)

Expand Down