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

Disable the fields of all CRUD workflow actions on readonly mode #9939

Merged
merged 11 commits into from
Jan 31, 2025

Conversation

Devessier
Copy link
Contributor

@Devessier Devessier commented Jan 30, 2025

Fixes https://discord.com/channels/1130383047699738754/1333822806504247467

In this PR:

  • Make the workflow step title input readonly when the visualizer is in readonly mode
  • Make all the fields of the Update Record and Delete Record readonly when the visualizer is in readonly mode
  • Create stories for the Create Record, Updated Record and Delete Record actions; I'm checking for the default mode and several variants of the disabled mode
  • Set up mocks for the workflows and use them in msw handlers

Follow up:

  • We use readonly and disabled alternatively; these are two different states when talking about a HTML <input /> element. I think we should settle on a single word.
  • Refactor the <WorkflowSingleRecordPicker /> component to behave as other selects
Current component Should look like
CleanShot 2025-01-30 at 17 30 29@2x CleanShot 2025-01-30 at 17 30 49@2x

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR implements readonly functionality across workflow action forms, focusing on consistent form field behavior in readonly mode.

Key changes:

  • Added disabled state to WorkflowStepHeader component for readonly workflow visualizer mode
  • Implemented comprehensive Storybook test coverage with stories for Create/Update/Delete Record actions in both default and disabled states
  • Added testId props throughout components for improved testability
  • Created WorkflowStepDecorator to set up workflow state for component testing
  • Introduced mock data and GraphQL handlers for workflow-related operations

Note: The PR raises an important issue about standardizing terminology between 'readonly' and 'disabled' states across components, which should be addressed in follow-up work.

15 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings | Greptile

@bosiraphael bosiraphael self-requested a review January 30, 2025 16:53
Copy link
Contributor

@bosiraphael bosiraphael left a comment

Choose a reason for hiding this comment

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

LGTM, left a couple of comments

@Devessier Devessier merged commit d946cdc into main Jan 31, 2025
47 checks passed
@Devessier Devessier deleted the disable-all-inputs-workflow-drawer branch January 31, 2025 11:31
Copy link

Fails
🚫

node failed.

Log

�[31mError: �[39m SyntaxError: Unexpected token C in JSON at position 0
    at JSON.parse (<anonymous>)
�[90m    at parseJSONFromBytes (node:internal/deps/undici/undici:5591:19)�[39m
�[90m    at successSteps (node:internal/deps/undici/undici:5562:27)�[39m
�[90m    at fullyReadBody (node:internal/deps/undici/undici:1665:9)�[39m
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m
�[90m    at async specConsumeBody (node:internal/deps/undici/undici:5571:7)�[39m
danger-results://tmp/danger-results-2d52eb07.json

Generated by 🚫 dangerJS against 9ef6f2b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants