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

Cannot save new job if editor panel is used to create name #1001

Closed
taylordowns2000 opened this issue Jul 31, 2023 · 2 comments · Fixed by #1006
Closed

Cannot save new job if editor panel is used to create name #1001

taylordowns2000 opened this issue Jul 31, 2023 · 2 comments · Fixed by #1006
Assignees
Labels
bug Newly identified bug

Comments

@taylordowns2000
Copy link
Member

https://www.loom.com/share/410646fee7404be4b35e9c8a7c61c49a

@taylordowns2000 taylordowns2000 added bug Newly identified bug elixir/phoenix labels Jul 31, 2023
@stuartc stuartc self-assigned this Aug 1, 2023
@stuartc
Copy link
Member

stuartc commented Aug 1, 2023

@taylordowns2000 this is a tough one.

The reason it's not saving is because the job has a nil body. The body is populated with a default only when the placeholder is accepted.

However the way we distinguish a placeholder from another job node is by checking for the existence of a name and body...

An intentional side effect of the syncing design is that it's really difficult to tell the difference between the client and server.

@josephjclark maybe I could get your advice here? I'm happy to keep the defaults in the front end (btw we do inject the default adaptor in a sync response fwiw - maybe we move that to the FE as well). I think we need to track the placeholder in the components state rather than derive which node is a placeholder; but this feels like it could be a sticky/tedious rearrange of the createNewWorkflow and identify functions...

I think this catch 22 between having a default body (or any other attrs) from the start will go away if we can decide which node is a placeholder and track that internally.

@stuartc
Copy link
Member

stuartc commented Aug 1, 2023

Notes from chat with @josephjclark

  1. Proposing we don't send a placeholder job to the server
  2. We show a disabled/empty form, that doesn't update as you type the name into the placeholder - it's just a blank box with some info.
  3. Using a m=placeholder query string to control showing this box.
  4. You won't be able to edit a Job until you commit the placeholder.

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

Successfully merging a pull request may close this issue.

3 participants