Skip to content

Comments

Feature/recipe input form#74

Merged
ascibisz merged 16 commits intomainfrom
feature/recipe-input-form
Sep 9, 2025
Merged

Feature/recipe input form#74
ascibisz merged 16 commits intomainfrom
feature/recipe-input-form

Conversation

@ascibisz
Copy link
Contributor

@ascibisz ascibisz commented Sep 3, 2025

Problem

Only certain fields of a recipe should be editable, not the whole JSON blob
Link to story or ticket

Solution

Editing the recipe should be like a form, with the editable fields that are specified in this spreadsheet

Some decisions I made, at least for now:

  • Changes do not persist when a different recipe is selected. Whenever a recipe is selected from the dropdown, the editable fields will show their default values. So if a user had previously edited a field for recipe A, switched to recipe B, and then went back to recipe A, their edit will no longer be there as recipe A will have all fields as their default values

Type of change

  • New feature (non-breaking change which adds functionality)

@github-actions
Copy link

github-actions bot commented Sep 3, 2025

PR Preview Action v1.6.2
Preview removed because the pull request was closed.
2025-09-09 21:29 UTC

@ascibisz ascibisz changed the base branch from main to feature/antd-components September 3, 2025 21:23
Base automatically changed from feature/antd-components to main September 3, 2025 22:13
@ascibisz ascibisz force-pushed the feature/recipe-input-form branch from f1db303 to 85fb02f Compare September 5, 2025 19:50
@ascibisz ascibisz marked this pull request as ready for review September 8, 2025 22:35
@ascibisz ascibisz requested a review from rugeli September 9, 2025 16:29
<Slider
min={gradientStrengthData.min}
max={gradientStrengthData.max}
defaultValue={gradientStrengthData.default}
Copy link
Contributor

Choose a reason for hiding this comment

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

Tracing the code, I'm still a bit unclear about how this prop works. Seems like we're using value={sliderValue} to control the display value, and the actual reset happens through setSliderValue which looks for strengthData.default. If that's the case, this line defaultValue={gradientStrengthData.default} is ignored and we can just remove it?

Copy link
Contributor Author

@ascibisz ascibisz Sep 9, 2025

Choose a reason for hiding this comment

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

just tested and it seems like we can remove that!

  • remove default value

Copy link
Contributor

@rugeli rugeli left a comment

Choose a reason for hiding this comment

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

Looks good! Just have one question, but I don't see any other issues with values on the preview site. Approving now, we can always circle back for improvements or Q&A later as our time is tight. Great work on this!!
One small thing: would it be helpful to round the display gradient strength value?
for reference
Screenshot 2025-09-09 at 10 01 26 AM

@ascibisz ascibisz merged commit 385090b into main Sep 9, 2025
2 checks passed
@ascibisz ascibisz deleted the feature/recipe-input-form branch September 9, 2025 21:29
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