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

Search-by-TFM feature flight UI bug fix #9357

Merged
merged 3 commits into from
Jan 24, 2023
Merged

Conversation

advay26
Copy link
Member

@advay26 advay26 commented Jan 24, 2023

Part of https://github.com/NuGet/Engineering/issues/4579

Problem:
When the Search-by-TFM feature flight was disabled for a user, as it will be for most users to begin with, the 'sort by' dropdown didn't automatically submit the form when a user changed its value.

Fix:
The new frameworks and tfms input fields in the html form were hidden behind the feature flight but still being referenced by some javascript code, which caused errors and prevented all the javascript code below it in the file from running. This code included the sort by event handler.

The frameworks and tfms were hidden form fields, so I've just moved them outside the feature flight to avoid errors. There is no visible change.

I've also moved the framework checkbox initialization code to the bottom of the javascript file so that it can't cause similar issues with other code.

I've also added a title attribute to one of the html links to fix an a11y issue I noticed.

@advay26 advay26 requested a review from a team as a code owner January 24, 2023 18:00
@advay26 advay26 changed the title Search-by-TFM Feature Flight UI Bug Fix Search-by-TFM feature flight UI bug fix Jan 24, 2023
@advay26 advay26 merged commit debc8a9 into dev Jan 24, 2023
@advay26 advay26 deleted the dev-advay26-featureflightbug branch January 24, 2023 21:16
@agr agr mentioned this pull request Jan 26, 2023
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants