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

Click to edit history name in HistoryPanel #19665

Merged
merged 10 commits into from
Mar 12, 2025

Conversation

ahmedhamidawan
Copy link
Member

@ahmedhamidawan ahmedhamidawan commented Feb 20, 2025

Fixes #19602

Screen.Recording.2025-02-20.at.8.06.09.PM.mov

Note: The input field looks a lot better now (similar to the one in the edit form we had before this)

Removed the name input field from editing section (only annotation and tags)

image

We already have the ability to edit by clicking the name, so no need for another input field for the same purpose.

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.

@ahmedhamidawan
Copy link
Member Author

ahmedhamidawan commented Feb 23, 2025

Should be good to go now, the failing tests are unrelated:

FAILED test/integration/test_interactivetools_api.py::TestInteractiveToolsIntegration::test_simple_execution - galaxy.tool_util.verify.wait.TimeoutAssertionError: Timed out after 60.25 seconds waiting on realtime hosted content at [http://2n3l6c82yy5ah-3pw29u3tq7f3s.ep.interactivetool.localhost:8493/.](http://2n3l6c82yy5ah-3pw29u3tq7f3s.ep.interactivetool.localhost:8493/)

FAILED test/integration/test_interactivetools_api.py::TestInteractiveToolsShortURLIntegration::test_simple_execution - galaxy.tool_util.verify.wait.TimeoutAssertionError: Timed out after 60.25 seconds waiting on realtime hosted content at [http://2n3l6c82yy5ah-13yxxwjo7for2.ep.interactivetool.localhost:9158/.](http://2n3l6c82yy5ah-13yxxwjo7for2.ep.interactivetool.localhost:9158/)

FAILED test/integration_selenium/test_edam_tool_panel_views.py::TestEdamToolPanelViewsSeleniumIntegration::test_basic_navigation - assert 0 > 0
 +  where 0 = len([])

@jdavcs
Copy link
Member

jdavcs commented Feb 25, 2025

I played with this and came up with a few thoughts. Sorry, its quite lengthy!

Consider these scenarios:

  1. Click the history name, modify it, then click away (anywhere on the screen) => the history name is saved (probably not what I expect when I click away; I would expect that to happen only when I either click the save icon or hit enter)
  2. I want to edit the history name, the first thing I think of is clicking the edit icon. I click it, but the history name is not editable. Clicking the name itself seems not obvious given that I've already clicked an edit icon which displayed a textbox for annotation.
  3. I clicked the edit icon, but was able to figure out that to edit the name I need to click the name. I do that, then edit the name, then change my mind and hit cancel => but the name is updated! (because see p.1: any click outside the box updates it)
  4. I clicked the edit icon and then clicked the history name. I have the "save" button and the save icon: which one do I click? One will save the name, the other will save all edits including the name, which is confusing, I think.
  5. I clicked the name to edit it, but I also want to edit the rest - so I click the edit icon => now I cannot edit the name anymore.

How about something like this (just ideas - may or may not work):

  • do not display an additional save icon when the name is clicked
  • if the name is clicked, update it only when the enter key is clicked.
  • maybe hide the edit icon when the name is being edited? Maybe replace it with the little save icon? If not, then if that edit icon is clicked, do not update the name.
  • if the edit icon is clicked, maybe do what the current UI is doing? (i.e., do not turn the history title into a textbox). That way, clicking the history name will be a convenient editing "shortcut", but will not interfere with the main editing UI.

@ahmedhamidawan
Copy link
Member Author

@jdavcs Thank you for the detailed review! Your points make sense. Making changes locally... Sorry for the late response, will update this comment once I push the local changes.

@ahmedhamidawan ahmedhamidawan force-pushed the rename_history_on_click branch from d92b84b to 8ff5105 Compare March 10, 2025 19:25
@ahmedhamidawan
Copy link
Member Author

@jdavcs

Click the history name, modify it, then click away (anywhere on the screen) => the history name is saved (probably not ...

f934998 addresses this

For the rest, I think this commit has a comprehensive and very straightforward solution 880910d :

rename_history_on_click.mp4
  • In general, its click to edit history name
  • Unless you click editing icon:
    • Which is when the existing editor opens up, and now clicking the name does nothing and you can change the name in the editor

Copy link
Member

@jdavcs jdavcs left a comment

Choose a reason for hiding this comment

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

I think this makes it a lot more convenient and straightforward. Very neat, thank you!

Copy link
Member

@dannon dannon left a comment

Choose a reason for hiding this comment

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

Good improvement, thanks @ahmedhamidawan -- failing tests are unrelated to the changes here.

@dannon dannon merged commit 1fb9cd4 into galaxyproject:dev Mar 12, 2025
52 of 57 checks passed
@ahmedhamidawan ahmedhamidawan deleted the rename_history_on_click branch March 12, 2025 21:57
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.

Renaming history requires a lot of clicks
3 participants