Skip to content

feat: Forcibly show a generic prompt dialog - #194

Open
ildar170975 wants to merge 7 commits into
iantrich:betafrom
ildar170975:prompt
Open

feat: Forcibly show a generic prompt dialog#194
ildar170975 wants to merge 7 commits into
iantrich:betafrom
ildar170975:prompt

Conversation

@ildar170975

Copy link
Copy Markdown
Collaborator

Replacing #186.
Allows to show a generic prompt dialog in cases like #177

@ildar170975

Copy link
Copy Markdown
Collaborator Author

@iantrich
Cannot pass "Claude Code Review" task, can you assist me with passing it?

… action an explicit GitHub token so it doesn't attempt the OIDC exchange to generate one internally.

2. Changed `pull-requests: read` → `pull-requests: write` — the review action needs write access to post review comments on PRs.
The root cause is that the action's `setupGitHubToken` function tries OIDC first (to get a GitHub token via token exchange), but `ACTIONS_ID_TOKEN_REQUEST_URL` isn't always set — particularly for PRs from forks or in certain runner environments. Providing the token explicitly bypasses that code path entirely.
@iantrich

iantrich commented May 22, 2026

Copy link
Copy Markdown
Owner

@ildar170975 rebase off beta. added a fix so forks can use the workflow.

@ildar170975
ildar170975 changed the base branch from beta to master May 22, 2026 00:50
@ildar170975
ildar170975 changed the base branch from master to beta May 22, 2026 00:51
@ildar170975

Copy link
Copy Markdown
Collaborator Author

rebase off beta. added a fix so forks can use the workflow.

Sorry, not sure I did it properly, now the PR includes changes in claude-code-review.yml ((.

@iantrich

Copy link
Copy Markdown
Owner

@claude do a code review on this PR, please

@claude

claude Bot commented May 22, 2026

Copy link
Copy Markdown

Claude finished @iantrich's task in 1m 50s —— View job


Code Review — PR #194: feat: Forcibly show a generic prompt dialog

  • Gather context and understand the request
  • Read changed files and diff against origin/beta
  • Analyze code changes
  • Post review findings

Summary: This PR adds a force_generic_dialog boolean option to bypass the HA custom pin-entry dialog and fall back to the browser's native prompt(). The implementation is minimal and focused. A few things worth discussing before merge:


src/restriction-card.ts — Core logic change

The actual change is a single condition guard at line 406:

// Before
if (this._helpers?.showEnterCodeDialog) {
// After
if (this._helpers?.showEnterCodeDialog && !this._config.force_generic_dialog) {

This is clean and correct. When the flag is set, the check short-circuits and falls through to the existing prompt() fallback. No regressions possible in the common path.

Note: force_generic_dialog only affects PIN input. The confirm restriction (lines 491–499) has an identical pattern but is not affected by this flag:

if (this._helpers?.showConfirmationDialog) {
  isConfirmed = await this._helpers.showConfirmationDialog(this, {...});
} else {
  isConfirmed = confirm(confirmText);
}

This appears intentional given the README description says "input a pin", but the option name force_generic_dialog implies broader scope. See naming note below.


src/editor.ts — UI toggle

The switch is added cleanly to the options section (line 261). Minor concern: when the switch is toggled off, _switchChanged passes false to _updateTop, which stores force_generic_dialog: false explicitly in the config rather than removing the key (since false doesn't match the undefined | null | '' deletion check). This is consistent with existing behavior for the row option — so not a regression — but users who enable then disable will see force_generic_dialog: false in their YAML config.

Label readability: "Force generic dialog" is cryptic to users. A label like "Use browser prompt for PIN (not HA dialog)" would better communicate what the option does. Fix this →


src/types.ts — Type definition

The field is correctly typed as force_generic_dialog?: boolean — no issues.


README.md — Documentation

The entry is added correctly to the options table. The description "Forcibly show a generic prompt popup dialog to input a pin" accurately scopes it to PIN input, which helps offset the naming ambiguity.


.github/workflows/claude-code-review.yml — Unrelated change

This file change crept in from the rebase and doesn't belong to this feature. The diff shows pull-requests: write and github_token additions — these are from the upstream master branch. Since the PR author acknowledged this, the maintainer can either:

  • Ask the author to clean up the branch to exclude this file, or
  • Merge as-is since the workflow change is already in master and won't cause harm.

Summary

Core logic ✅ Correct and minimal
Types ✅ Properly typed
Editor UI ⚠️ Label "Force generic dialog" is unclear; consider more descriptive text
Naming ⚠️ force_generic_dialog sounds broader than it is (PIN only); force_pin_prompt would be clearer
false in config ℹ️ Minor — consistent with existing pattern for row
Confirm restriction ℹ️ Not affected by flag — acceptable if intentional
Workflow file ⚠️ Unrelated change from rebase — @iantrich to decide whether to ask for cleanup

The feature itself works correctly. The two naming/labeling issues are the most worth addressing before merge.

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.

2 participants