Skip to content

feat(catalogs): allow binding to openUrl url#1453

Open
tlecomte wants to merge 2 commits into
google:mainfrom
tlecomte:urlBinding
Open

feat(catalogs): allow binding to openUrl url#1453
tlecomte wants to merge 2 commits into
google:mainfrom
tlecomte:urlBinding

Conversation

@tlecomte
Copy link
Copy Markdown

@tlecomte tlecomte commented May 19, 2026

Description

Allow the url of the openUrl function to bind to a path in the data model.

This allows to have buttons in a list of cards that are bound to a url in a datamodel.

Fixes #1390.

Pre-launch Checklist

If you need help, consider asking for advice on the discussion board.

@google-cla
Copy link
Copy Markdown

google-cla Bot commented May 19, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the url property definition across several JSON catalog files to support dynamic strings by incorporating a reference to DynamicString and applying conditional URI formatting. It also replaces additionalProperties with unevaluatedProperties in the affected schemas. Feedback was provided regarding the lack of test cases for these changes, which violates the repository style guide's requirement that code changes must include tests to verify new validation logic.

Comment on lines +1001 to +1013
"allOf": [
{
"$ref": "common_types.json#/$defs/DynamicString"
},
{
"if": {
"type": "string"
},
"then": {
"format": "uri"
}
}
],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The PR description indicates that no tests were added for these changes, and the pre-launch checklist for tests is unchecked. According to the Repository Style Guide (line 17), code changes should have tests.

Please add test cases (e.g., in specification/v0_10/test/cases/) to verify that the url property correctly validates both literal URIs and dynamic bindings (data paths and function calls), and that invalid literal strings still fail validation as expected.

References
  1. If there are code changes, code should have tests. (link)

@tlecomte tlecomte marked this pull request as ready for review May 20, 2026 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

Why does openUrl not support binding dynamic data path

1 participant