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

Update labels in Markdown editor when workflow labels change. #17863

Merged
merged 9 commits into from
Apr 2, 2024

Conversation

jmchilton
Copy link
Member

Extends the work of @assuntad23 and myself in #16864 and #17264 to handle spaces and quotes in labels better and to include workflow output labels in addition to input and step labels.

How to test the changes?

(Select all options that apply)

  • Instructions for manual testing are as follows:
    1. Create a workflow with a label on one of the steps
    2. Go to edit the Report, and create an item in the report that references a label
    3. Navigate back to workflow steps, and change the label.
    4. Go back to the report and see that the label is changed.

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@jmchilton jmchilton force-pushed the update-label-markdown-editor branch from cfae4a5 to df340f5 Compare March 28, 2024 19:37
@jmchilton jmchilton marked this pull request as ready for review March 28, 2024 20:48
@github-actions github-actions bot added this to the 24.1 milestone Mar 28, 2024
Copy link
Contributor

@davelopez davelopez left a comment

Choose a reason for hiding this comment

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

Pretty awesome! thank you so much for the tests!

It works pretty well! The only thing I found while testing is that it still needs to handle spaces in the label:

WfStepRenameIssue

@jmchilton
Copy link
Member Author

@davelopez Thanks for the screencast - I can reproduce the problem but I wrote unit tests so this seems unfair. I think the GUI should what the tests say it should 😆. I'll work on this next week - thanks for the review so much.

@jmchilton jmchilton marked this pull request as draft March 30, 2024 22:21
@jmchilton
Copy link
Member Author

Okay - the function fails if labels contain spaces at the end... which they invariably will briefly if using a keyboard to type them in and we update on each keystroke. The missing test is obvious so that is great - I'll try to fix it now.

@jmchilton jmchilton force-pushed the update-label-markdown-editor branch from df340f5 to beeaa27 Compare April 1, 2024 17:08
@jmchilton jmchilton marked this pull request as ready for review April 1, 2024 20:51
@jmchilton
Copy link
Member Author

Okay - I think I fixed the problem in the screenshot view! Thanks again for the detailed review @davelopez

Copy link
Contributor

@davelopez davelopez left a comment

Choose a reason for hiding this comment

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

Thank you! it works nicely!

client/src/components/Markdown/parse.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@assuntad23 assuntad23 left a comment

Choose a reason for hiding this comment

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

@davelopez davelopez merged commit c253a99 into galaxyproject:dev Apr 2, 2024
30 checks passed
@martenson martenson deleted the update-label-markdown-editor branch April 2, 2024 20:23
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.

3 participants