Skip to content

Fix: uncontrolled input in dashboard - #895

Closed
Animesh404 wants to merge 1 commit into
OpenAdaptAI:mainfrom
Animesh404:fix/entry-point-warning
Closed

Fix: uncontrolled input in dashboard#895
Animesh404 wants to merge 1 commit into
OpenAdaptAI:mainfrom
Animesh404:fix/entry-point-warning

Conversation

@Animesh404

@Animesh404 Animesh404 commented Oct 26, 2024

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

a Bugfix for #889

Summary

  • Removed checked property: This avoids conflicting control by letting getInputProps fully manage the checkbox state.
  • Added defaultSettings which provides default values for all fields, preventing uncontrolled-to-controlled issues.

Checklist

  • My code follows the style guidelines of OpenAdapt
  • I have performed a self-review of my code
  • If applicable, I have added tests to prove my fix is functional/effective
  • I have linted my code locally prior to submission
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (e.g. README.md, requirements.txt)
  • New and existing unit tests pass locally with my changes

How can your code be run and tested?

poetry shell
python -m openadapt.entrypoint

OPENAI_API_KEY: '',
ANTHROPIC_API_KEY: '',
GOOGLE_API_KEY: '',
};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks like the third place we are duplicating configuration setting names. Is there any way to populate this dynmamically?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

we populate this dynamically using config file but sometimes we might get undefined values which might be the reason for warning. So, I added these default values

<form onSubmit={form.onSubmit(saveSettings(form))}>
<Grid>
<Grid.Col span={6}>
<Checkbox label="Record window data" {...form.getInputProps('RECORD_WINDOW_DATA')} checked={form.values.RECORD_WINDOW_DATA} />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Won't this prevent the checkboxes from being checked appropriately depending on the configuration values?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

...form.getInputProps('RECORD_WINDOW_DATA') this alone should've marked this checked but yes now that I tested it again without checked= property it is not being marked appropriately.

@abrichr

abrichr commented Jan 17, 2026

Copy link
Copy Markdown
Member

Closing as part of the architecture transition. The legacy codebase has been moved to legacy/ and frozen at v0.46.0.

If this change is still needed, please:

  1. Open a PR against the relevant modular package repository
  2. Or re-open this PR targeting the legacy/ directory if it's a critical legacy fix

See PR #960 for details on the new meta-package architecture.

@abrichr abrichr closed this Jan 17, 2026
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