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

Temporarily revise the valid combinations of ATs and Browsers for embed.js #650

Merged
merged 2 commits into from
Jun 6, 2023

Conversation

howard-e
Copy link
Contributor

@howard-e howard-e commented Jun 5, 2023

Based on #583.

I've revised the getAllAtBrowserCombinations function after noticing that there's a case where the function may cause the /embed url to incorrectly display a AT/Browser pairing as Not Applicable when it should say Data Not Yet Available instead.

I've noted this as a temporary patch as it only applies for this file and having a useable solution to be used elsewhere in the application would be best. It most recently aligns with requirements described in /648.

This is currently being tracked in #625

@howard-e howard-e requested a review from alflennik June 5, 2023 19:26
Copy link
Contributor

@alflennik alflennik left a comment

Choose a reason for hiding this comment

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

Code looks good - I agree that in the future it might be nice to have this data in the resolvers, but actually for the immediate future calculating these values in controller actually seems pretty clean.

I ran it and verified that the data is showing up as intended.

@howard-e howard-e merged commit d062676 into main Jun 6, 2023
@howard-e howard-e deleted the support-tables-not-applicable-2 branch June 6, 2023 16:33
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.

2 participants