Skip to content

docs: document depends_on conditional visibility for settings - #311

Merged
nicomiguelino merged 3 commits into
masterfrom
feat/edge-app-settings-conditional-visibility
Aug 25, 2026
Merged

docs: document depends_on conditional visibility for settings#311
nicomiguelino merged 3 commits into
masterfrom
feat/edge-app-settings-conditional-visibility

Conversation

@nicomiguelino

Copy link
Copy Markdown
Contributor

Summary

  • Documents the new properties.depends_on key in a setting's help_text schema, which the web console (Screenly/Screenly#2554) now supports for conditional visibility: a setting only renders (and is submitted) while another named setting's current value is one of a listed set of values.
  • Adds a "Conditional visibility" example alongside the existing input field type examples.
  • No Rust changes needed: help_text stays an opaque string to the CLI, exactly like type/options/advanced already are.

Copilot AI lite review requested due to automatic review settings August 25, 2026 10:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The web console now supports a depends_on: { setting, values } key
inside a setting's help_text schema so it only renders (and is
submitted) while another setting's current value is one of the
listed values. Purely a frontend-parsed convention, same as type/
options/advanced — no Rust changes needed since help_text stays an
opaque string to the CLI.
Copilot AI review requested due to automatic review settings August 25, 2026 10:11
@nicomiguelino
nicomiguelino force-pushed the feat/edge-app-settings-conditional-visibility branch from 6beb0b8 to 909008b Compare August 25, 2026 10:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sergey-borovkov sergey-borovkov left a comment

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.

Docs read well and the example is clear. One gap I'd like closed before merging.

depends_on on a required field is undefined

docs/EdgeApps.md:486

The bullet says a hidden field's value "isn't collected or written on save", and the example correctly uses optional: true — but nothing states whether optional: false combined with depends_on is a supported combination.

A reader who marks a conditional field required gets an install form that may fail validation on a field they cannot see, with no way to work out why from these docs. One sentence either saying conditional fields should be optional: true, or describing how required-ness is skipped while the field is hidden, closes it.


Note for merge ordering: this PR and #308 both add a bullet to the same list at docs/EdgeApps.md:486 and a section at the same anchor, so expect a textual conflict on whichever lands second. No semantic conflict — #308's help_text_with_display_order only inserts into an existing schema's properties, so it preserves depends_on.

Copilot AI review requested due to automatic review settings August 25, 2026 15:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sergey-borovkov sergey-borovkov left a comment

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.

The added sentence covers it — thanks. Approving.

Heads up that this will still conflict textually with #308 at docs/EdgeApps.md:486 (same list, same anchor); whichever lands second needs a trivial rebase. No semantic conflict.

@nicomiguelino
nicomiguelino merged commit b309cba into master Aug 25, 2026
4 of 5 checks passed
@nicomiguelino
nicomiguelino deleted the feat/edge-app-settings-conditional-visibility branch August 25, 2026 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants