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.

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

[1/n] Global parameters renderer #673

Merged
merged 1 commit into from
Dec 29, 2023
Merged

[1/n] Global parameters renderer #673

merged 1 commit into from
Dec 29, 2023

Conversation

saqadri
Copy link
Contributor

@saqadri saqadri commented Dec 29, 2023

[1/n] Global parameters renderer

Use ParametersRenderer to modify global parameters.

Currently still updating the entire parameters object. Next up, will fix the ParametersRenderer component to update each parameter separately.

@saqadri
Copy link
Contributor Author

saqadri commented Dec 29, 2023

Test Plan:

pip install -e python/
alias aiconfig="python3 -m 'aiconfig.scripts.aiconfig_cli'"
aiconfig edit --aiconfig-path=/Users/saqadri/lm/aiconfig/python/src/aiconfig/editor/travel.aiconfig.json --server-port=8080 --server-mode=debug_servers
Screen.Recording.2023-12-29.at.1.35.38.PM.mov

Copy link
Contributor

@rholinshead rholinshead left a comment

Choose a reason for hiding this comment

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

Functionality LGTM and we can iterate on the UI itself to better match the designs

initialValue,
onUpdateParameters,
}: Props) {
const [isParametersDrawerOpen, setIsParametersDrawerOpen] = useState(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the LOE to make this component UI into the card with bottom show/hide like in the figma -- https://www.figma.com/file/7YVa6KpEDgptj8DiAc5BaU/AiConfig-Editor?type=design&node-id=328-5037&mode=design&t=h3xAGY47py1mg4Fk-0 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not very high. I can do that as a follow-up next week

Use ParametersRenderer to modify global parameters.

Currently still updating the entire parameters object. Next up, will fix the ParametersRenderer component to update each parameter separately.
@saqadri saqadri merged commit 5bcfd70 into main Dec 29, 2023
3 checks passed
saqadri added a commit that referenced this pull request Dec 29, 2023
[easy] Simplify update parameters callback

Have updateParameters take a JSONObject. Makes it much cleaner to use
elsewhere.

ParametersRenderer simply converts its ParametersArray to JSONObject
before triggering the update.

---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/674).
* __->__ #674
* #673
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