Skip to content

Removes outdated Markdown visualization insertion wizard #19898

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

Merged
merged 5 commits into from
Apr 14, 2025

Conversation

guerler
Copy link
Contributor

@guerler guerler commented Mar 25, 2025

This PR removes the outdated visualizations wizard from the text-based markdown editor. Visualizations can be inserted using the new cell-based Markdown editor, instead. Moving forward my current plan is to remove the side panel from the text-based editor and instead re-use the insertion dropdown from the cell-based editor. For now removing just the outdated visualization insertion wizard is sufficient.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

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

@guerler guerler added kind/bug area/UI-UX kind/refactoring cleanup or refactoring of existing code, no functional changes labels Mar 25, 2025
@guerler guerler marked this pull request as ready for review March 25, 2025 16:34
@github-actions github-actions bot added this to the 25.0 milestone Mar 25, 2025
@mvdbeek
Copy link
Member

mvdbeek commented Mar 26, 2025

I tried the cell based thing today and I don't like using it. Can't remove a cell, can't re-arrange a cell, can't copy a cell, can't find a cell, can't replace a cell. All of that makes me wonder why did you go for cells in the first place instead of rendering a nicer preview ? It's not like we're doing computations. For practical purposes the cell-based editor is not ready, so I would not remove the text based one yet.

@guerler
Copy link
Contributor Author

guerler commented Mar 26, 2025

I tried the cell based thing today and I don't like using it. Can't remove a cell, can't re-arrange a cell, can't copy a cell, can't find a cell, can't replace a cell. All of that makes me wonder why did you go for cells in the first place instead of rendering a nicer preview ? It's not like we're doing computations. For practical purposes the cell-based editor is not ready, so I would not remove the text based one yet.

@mvdbeek have you tried clicking on the 3 dots in the guide bar?
image

Also this PR does not remove the text-based one and the text-based one will remain the default view. Instead this PR removes the now outdated visualization insertion option. The previous visualization insertion used to build a Galaxy form to simplify configuring the visualization. This not necessary anymore since the input form is rendered by the visualization itself.

@mvdbeek
Copy link
Member

mvdbeek commented Mar 26, 2025

I did find it eventually, but this is not intuitive at all, and if you want to re-arrange sections you'll be busy for a very long time. Also writing in the little cells does not feel nice.

@guerler
Copy link
Contributor Author

guerler commented Mar 26, 2025

@mvdbeek fair enough, we can make adjustments regarding layout and visibility moving forward e.g. allowing drag-and-drop, resizable code and element sections etc. Ultimately, regardless of layout the editors will rely on the same core mechanism and underlying modules, offering identical functionality. The only difference lies in their layout: the default/text-based editor will display the full text alongside a separate preview, likely using a split-screen view, while the cell-based editor will present the preview followed by the corresponding code/text section, but this is unrelated to the purpose of this PR. The visualization insertion form in the text-based editor is outdated, not functional and needs to be removed. Once we have the Monaco editor in the code-base, I intend to re-use the insertion component from the cell-based editor.

@jmchilton
Copy link
Member

Once we have the Monaco editor in the code-base, I intend to re-use the insertion component from the cell-based editor.

This is really heartening. I get there is some good short term wins using cells but I think we will be better off in the long term aiming for more organic documents.

@jmchilton jmchilton merged commit 4f65729 into galaxyproject:dev Apr 14, 2025
32 of 33 checks passed
@guerler guerler deleted the remove_legacy_viz_insert branch April 14, 2025 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/UI-UX kind/bug kind/refactoring cleanup or refactoring of existing code, no functional changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants