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

Replace custom tour ids in forms with regular DOM identifiers #13252

Merged
merged 4 commits into from
Jan 28, 2022

Conversation

guerler
Copy link
Contributor

@guerler guerler commented Jan 26, 2022

Requires #13246. This PR removes the custom tour_id dom attribute and uses the regular id attribute instead to query form input elements. It also removes the input id replacement of |. Although a special character newer conventions do allow its use as part of a dom id. A benefit of this PR is that input element identifiers for dom elements are now equivalent to actual parameter identifiers used by the api and elsewhere.

How to test the changes?

  • This is a refactoring of components with existing test coverage.

License

@guerler guerler added area/UI-UX kind/refactoring cleanup or refactoring of existing code, no functional changes labels Jan 26, 2022
@guerler guerler added this to the 22.01 milestone Jan 26, 2022
@jmchilton
Copy link
Member

element-id seems redundant and generic as a prefix for a DOM ID. My preference would be gxform-input-. But this is a small thing - I am okay with the way you have currently.

@guerler guerler force-pushed the remove_tour_id branch 4 times, most recently from a60c858 to 2b89c96 Compare January 28, 2022 05:05
@guerler
Copy link
Contributor Author

guerler commented Jan 28, 2022

@jmchilton I agree with you, it is redundant. I replaced it with form-element since it is the used as part of the FormElement component. Thank you for looking into it.

@guerler guerler changed the title [WIP] Replace custom tour ids in forms with regular DOM identifiers Replace custom tour ids in forms with regular DOM identifiers Jan 28, 2022
@guerler guerler force-pushed the remove_tour_id branch 2 times, most recently from 52d1d92 to a6d5bef Compare January 28, 2022 05:29
@dannon dannon modified the milestones: 22.01, 22.05 Jan 28, 2022
@guerler guerler marked this pull request as ready for review January 28, 2022 13:44
@dannon dannon modified the milestones: 22.05, 22.01 Jan 28, 2022
@dannon
Copy link
Member

dannon commented Jan 28, 2022

We're going to try to get this one into 22.01 to minimize conflicts with any bugfixes that may need to propagate forward.

@dannon dannon merged commit 4c35a01 into galaxyproject:dev Jan 28, 2022
@guerler guerler deleted the remove_tour_id branch January 28, 2022 23:10
@guerler
Copy link
Contributor Author

guerler commented Jan 28, 2022

Thank you for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/UI-UX 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