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

Improve styling and simplify workflow example #148

Merged
merged 4 commits into from
Jan 17, 2022
Merged

Conversation

planger
Copy link
Member

@planger planger commented Jan 7, 2022

@planger planger requested a review from tortmayr January 7, 2022 17:19
Copy link
Contributor

@ndoschek ndoschek left a comment

Choose a reason for hiding this comment

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

Thanks Philip!
While testing the workflow example, I recognized that two validation features do not work anymore for the Workflow example:

The label edit validation should prompt Name should be unique while typing. Currently, this only works for the Push node, as here the name and label text value are identical. In general, the name and label text values differ for most task nodes in the example workflow model.
As I could observe, the label edit changes the label's text and the validator checks the name of a task node instead. Could we align the behavior to get the correct label edit validation?

@planger planger requested a review from ndoschek January 11, 2022 20:13
Copy link
Contributor

@ndoschek ndoschek left a comment

Choose a reason for hiding this comment

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

Thanks for the update Philip! The label edit validation works now as expected! 🎉

Unfortunately I didn't elaborate my comment enough.
The validation in the WorkflowModelValidator (if a task name starts with an upper case letter) is also broken, because there is no dedicated compartment in the taskNode anymore (see validateTaskNode_labelStartsUpperCase ).

Could you have another look? Thanks!

@planger
Copy link
Member Author

planger commented Jan 14, 2022

Thanks for the great catch! I fixed the workflow validator in 453b347.

Copy link
Contributor

@ndoschek ndoschek left a comment

Choose a reason for hiding this comment

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

Great, thank you Philip for the quick fix! Looks great! 👍

I would have one final minor request, I just realized the file headers should be updated to -2022. Thanks!

@planger
Copy link
Member Author

planger commented Jan 14, 2022

Thanks, I updated the headers.

Copy link
Contributor

@ndoschek ndoschek left a comment

Choose a reason for hiding this comment

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

Thanks Philip! 🎉

@planger planger merged commit 415c0cb into master Jan 17, 2022
@planger planger deleted the planger/issues/492 branch January 17, 2022 08:19
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.

None yet

3 participants