8/n Read-only mode: Input rendering #962

Ankush-lastmile · 2024-01-18T18:59:00Z

8/n Read-only mode: Input rendering

Attempted to add read-only functionality wherever applicable to input renderings.

Testplan

dependencies

built ontop of #961

7/n Read-only mode: Prompt Menu (Left side triple dots) - Add ReadOnly prop to the Prompt Menu (triple dots on the left side of the component) ## Testplan 1. Pass readOnly=True prop in `AIConfigEditor` | readOnly={false} | readOnly={true} | | ------------- | ------------- | | <img width="1172" alt="Screenshot 2024-01-18 at 1 24 10 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/86842c3d-b864-45ef-bce4-77435c5d0518"> | <img width="1127" alt="Screenshot 2024-01-18 at 1 25 33 PM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/38c9ef53-dfe8-4eb8-b7f2-5f87adf2a708"> | --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/961). * #962 * __->__ #961 * #957

[easy][editor][client] 6/n Read-only mode: Global Parameters - refactor `isReadonly` -> `readOnly` inside `ParametersRenderer.tsx` - useState() to get readOnly bool ## Testplan 1. Pass readOnly=True prop in `AIConfigEditor` | readOnly={false} | readOnly={true} | | ------------- | ------------- | | <img width="981" alt="Screenshot 2024-01-18 at 11 29 08 AM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/07747d6a-37d1-4b00-a5fb-ae8475c19044"> | <img width="1044" alt="Screenshot 2024-01-18 at 11 32 28 AM" src="https://github.com/lastmile-ai/aiconfig/assets/141073967/673abdb3-7048-444c-9ba2-8ad746181478"> | ## Dependencies Built ontop of #939 #961 is next on the stack --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/967). * #962 * __->__ #967

Ankush-lastmile · 2024-01-19T00:52:24Z

.../client/src/components/prompt/prompt_input/schema_renderer/PromptInputDataSchemaRenderer.tsx

  switch (schema.type) {
    case "string":
      return (
        <Textarea
          value={data ? (data as string) : ""}
          onChange={(e) => onChangeData(e.target.value)}
+          disabled={readOnly}
          placeholder="Type a prompt"


should this say something else in readonly mode?

Placeholder is a value for when there is no data already and asking user to do an action. In our case let's set this to "" in readOnly mode (can you test example where the input string is empty) for both readOnly and normal? Thanks!

added spoiler

placeHolder no longer needed since spoiler will render input which will be an undefined/empty string

Ankush-lastmile · 2024-01-19T00:52:34Z

...itor/client/src/components/prompt/prompt_input/schema_renderer/PromptInputSchemaRenderer.tsx

@@ -77,6 +80,7 @@ export default memo(function PromptInputSchemaRenderer(props: Props) {
        label="Prompt"
        onChange={(e) => props.onChangeInput(e.target.value)}
        placeholder="Type a prompt"
+        disabled = {readOnly}


And here :p

Same about making placeholder empty string for readOnly mode

readOnly = False

readOnly = True

placeHolder no longer needed since spoiler will render input which will be an undefined/empty string

python/src/aiconfig/editor/client/src/components/JSONEditorToggleButton.tsx

rholinshead · 2024-01-19T02:04:18Z

.../src/aiconfig/editor/client/src/components/prompt/prompt_input/PromptInputConfigRenderer.tsx

  return (
    <Textarea
      value={input as string}
      onChange={(e) => onChangeInput(e.target.value)}
+      disabled={readOnly}


Instead of disabled textarea, let's render the nicer TextRenderer that text outputs use (for all places in this PR). Maybe we could also wrap that with a Spoiler in case the input is long (see TextInputCellRenderer in lastmile - https://github.com/lastmile-ai/lastmile/blob/bd3b54248682aed2720f7c2c3caefc8ebda335ee/src/components/playground_v2/cell_renderers/TextInputCellRenderer.tsx#L143-L156)

updated
readOnly = False

readOnly = True

rholinshead · 2024-01-19T02:06:52Z

.../src/components/prompt/prompt_input/schema_renderer/PromptInputAttachmentsSchemaRenderer.tsx

@@ -38,7 +40,7 @@ function EditableAttachmentRenderer({
          onRemoveAttachment={onRemoveAttachment}
          onEditAttachment={() => setShowUploader(true)}
        />
-      ) : (
+      ) : !readOnly && (
        <AttachmentUploader


If they have a prompt that takes attachments but it has none, will this just be a blank prompt? Might be better to just render the AttachmentUploader still and have the dropzone disabled to denote that it's an input for attachments?

I don't feel too strongly here -- can you compare how they look?

whoops sorry forgot to include this in the testplan.

vs

disabled doesn't change how its rendered but it disables the dropzone.

updated to dropzone, Thought it wouldn't be nice to show the dropzone,

I think the disabled dropzone is better than blank for now, otherwise there's no indication of what the input is 'supposed to be'. We do the same in lastmile so it is consistent as well

rholinshead · 2024-01-19T02:08:11Z

.../client/src/components/prompt/prompt_input/schema_renderer/PromptInputDataSchemaRenderer.tsx

  switch (schema.type) {
    case "string":
      return (
        <Textarea
          value={data ? (data as string) : ""}
          onChange={(e) => onChangeData(e.target.value)}
+          disabled={readOnly}


Same here r.e. using the TextRenderer w/ Spoiler

rholinshead · 2024-01-19T02:08:50Z

...itor/client/src/components/prompt/prompt_input/schema_renderer/PromptInputSchemaRenderer.tsx

@@ -77,6 +80,7 @@ export default memo(function PromptInputSchemaRenderer(props: Props) {
        label="Prompt"
        onChange={(e) => props.onChangeInput(e.target.value)}
        placeholder="Type a prompt"
+        disabled = {readOnly}


And here :p

rossdanlm · 2024-01-19T04:58:35Z

python/src/aiconfig/editor/client/src/components/JSONRenderer.tsx

@@ -14,7 +15,9 @@ export default memo(function JSONRenderer({
  onChange,
  schema,
 }: Props) {
-  return !onChange ? (
+  const { readOnly } = useContext(AIConfigContext);
+  // Prism is a read only renderer.


Nice comment!

rossdanlm

Accepting to unblock but pls land with Ryan and my comments!

Ankush-lastmile · 2024-01-19T15:41:41Z

Switch negation in JSONEditorToggleButton
Spoiler to PromptInputConfigRenderer
Spoiler in PromptInputSchemaRenderer
Spoiler in PromptInputSchemaRenderer.tsx
disabled dropzone on readOnly

Ankush-lastmile · 2024-01-19T15:46:18Z

remove readOnly on attachment renderer

Ankush-lastmile · 2024-01-19T15:51:06Z

Accepting to unblock but pls land with Ryan and my comments!

Addressed all comments, Shipping this diff. Will fix forward if anything else might have been missed

This was referenced Jan 18, 2024

7/n Read-only mode: Prompt Menu (Left side triple dots) #961

Merged

[easy][editor][client] 6/n Read-only mode: Global Parameters #957

Closed

Ankush-lastmile force-pushed the pr962 branch from 3300213 to 14351df Compare January 18, 2024 23:05

Ankush-lastmile mentioned this pull request Jan 18, 2024

[easy][editor][client] 6/n Read-only mode: Global Parameters #967

Merged

Ankush-lastmile force-pushed the pr962 branch from 14351df to 9917af9 Compare January 18, 2024 23:06

Ankush-lastmile force-pushed the pr962 branch 2 times, most recently from 4f5461e to 85accb3 Compare January 19, 2024 00:49

Ankush-lastmile changed the title ~~{wip} 8/n Read-only mode: Input rendering~~ 8/n Read-only mode: Input rendering Jan 19, 2024

Ankush-lastmile force-pushed the pr962 branch 2 times, most recently from 3b5d645 to 561a694 Compare January 19, 2024 00:52

Ankush-lastmile commented Jan 19, 2024

View reviewed changes

Ankush-lastmile marked this pull request as ready for review January 19, 2024 00:52

Ankush-lastmile requested review from saqadri, rholinshead, suyoglastmileai, jonathanlastmileai and rossdanlm as code owners January 19, 2024 00:52

rholinshead reviewed Jan 19, 2024

View reviewed changes

rossdanlm reviewed Jan 19, 2024

View reviewed changes

rossdanlm approved these changes Jan 19, 2024

View reviewed changes

Ankush-lastmile force-pushed the pr962 branch from 561a694 to 824a479 Compare January 19, 2024 15:39

Ankush-lastmile force-pushed the pr962 branch from 824a479 to f3c043c Compare January 19, 2024 15:45

Ankush-lastmile merged commit 2dbcf0a into main Jan 19, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8/n Read-only mode: Input rendering #962

8/n Read-only mode: Input rendering #962

Ankush-lastmile commented Jan 18, 2024 •

edited

Loading

Ankush-lastmile Jan 19, 2024

rossdanlm Jan 19, 2024

Ankush-lastmile Jan 19, 2024 •

edited

Loading

Ankush-lastmile Jan 19, 2024

rholinshead Jan 19, 2024

rossdanlm Jan 19, 2024

Ankush-lastmile Jan 19, 2024

rholinshead Jan 19, 2024 •

edited

Loading

Ankush-lastmile Jan 19, 2024

rholinshead Jan 19, 2024

rholinshead Jan 19, 2024

Ankush-lastmile Jan 19, 2024 •

edited

Loading

rholinshead Jan 19, 2024

Ankush-lastmile Jan 19, 2024

rholinshead Jan 19, 2024

Ankush-lastmile Jan 19, 2024

rholinshead Jan 19, 2024

rossdanlm Jan 19, 2024

Ankush-lastmile Jan 19, 2024

rossdanlm left a comment

Ankush-lastmile commented Jan 19, 2024 •

edited

Loading

Ankush-lastmile commented Jan 19, 2024

Ankush-lastmile commented Jan 19, 2024

8/n Read-only mode: Input rendering #962

8/n Read-only mode: Input rendering #962

Conversation

Ankush-lastmile commented Jan 18, 2024 • edited Loading

Testplan

dependencies

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ankush-lastmile Jan 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rholinshead Jan 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ankush-lastmile Jan 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rossdanlm left a comment

Choose a reason for hiding this comment

Ankush-lastmile commented Jan 19, 2024 • edited Loading

Ankush-lastmile commented Jan 19, 2024

Ankush-lastmile commented Jan 19, 2024

Ankush-lastmile commented Jan 18, 2024 •

edited

Loading

Ankush-lastmile Jan 19, 2024 •

edited

Loading

rholinshead Jan 19, 2024 •

edited

Loading

Ankush-lastmile Jan 19, 2024 •

edited

Loading

Ankush-lastmile commented Jan 19, 2024 •

edited

Loading