feat: add status message below prompt editor #284
Conversation
audrey-kho
requested review from
askayastha22,
irenelagno,
vtsaplin and
lenaorobei
| <Text> | ||
| The characters <span style={{ fontWeight: '600' }}>{</span>, <span style={{ fontWeight: '600' }}>}</span>, | ||
| and <span style={{ fontWeight: '600' }}>"</span> are reserved and can't be used within quoted text values. | ||
| Please remove or replace these characters and try again. | ||
| </Text> |
There was a problem hiding this comment.
Localizable text. It's a good opportunity to localize a very complex text string.
Please refer to this example:
There was a problem hiding this comment.
@audrey-kho , I left some comments, could you please take a look when you have time?
CC: @askayastha22
| <PromptEditor | ||
| isOpen={isOpenPromptEditor} | ||
| onClose={() => setIsOpenPromptEditor(false)} | ||
| error={promptEditorError} |
There was a problem hiding this comment.
From my understanding of the app's architecture, the PromptSessionPanel component might be considered a form component. It could have its own validation state, which other child components might call to notify about any validation errors. Child components like PromptSessionSideView and PromptEditor might have their own internal validation states to track their validation logic.
PromptSessionPanel (a form element) - has an internal state to track form validation and form controls
|-- PromptSessionSideView (an input element)
|-- PromptEditor (an input element) - has its own internal state to track its validation logic and notifies PromptSessionPanel in case of errors
- once
PromptEditorencounters a validation error, it should notifyPromptSessionPanel - in turn,
PromptSessionPanelshould notifyPromptSessionSideViewto disableGenerateButton
In short, passing promptEditorError isn't needed. PromptEditor should have its own state to control its validation logic and only notify PromptSessionPanel about errors by calling PromptSessionPanel's setPromptEditorError method. However, we need to rename it to something like setPromptValidationError since other child components that we could have might also fail their own validation logic.
However, I need input from @askayastha22 as well.
There was a problem hiding this comment.
- From my point of view, even though
PromptSessionPanellooks like a form element, it's dynamically created via prompt syntax. So, validation logic for each text field is hard to determine beforehand. - As for your point about
PromptEditorhaving its own state to control validation logic, it is a good catch and we may refactor the code.
Description
Related Issue
https://jira.corp.adobe.com/browse/SITES-21736
Motivation and Context
How Has This Been Tested?
https://experience-qa.adobe.com/?devMode=true&shell_source=workspace&workspace=charerrorui&shell_ims=prod#/aem/generate-variations/
Screenshots (if appropriate):
Types of changes
Checklist: