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

Fix for crashes when grid images are wider than the screen #2673

Merged
merged 1 commit into from
May 15, 2023

Conversation

ctsims
Copy link
Member

@ctsims ctsims commented May 12, 2023

Summary

Basic fix for this issue. Essentially, forms will crash if the image for a grid select view is wider in pixels than the native display.

Release Note:

  • Fix for a crash which could occur with compact image selection widgets on small screens with large images

Feature Flag

It is specific to an uncommon pattern involving the use of the compact attribute on <select> and <select1> questions with images on the select items, which displays a grid of images to choose from.

Product Description

N/A

Safety Assurance

  • If the PR is high risk, "High Risk" label is set
  • I have confidence that this PR will not introduce a regression for the reasons below
  • Do we need to enhance manual QA test coverage ? If yes, "QA Note" label is set correctly

Automated test coverage

I think we should consider seeing if we could usefully automate some tests around the image grid selection if we don't have them currently, as the project reporting this issue is quite high-value and high-impact, and they use the 'grid' pattern images in multiple places. Testing image widths and how the screen changes would be a valuable component of those tests.

Safety story

This fix has been tested locally after replicating the original issue. There's very small risk of introducing a regression due to the limited scope of the change.

Copy link
Contributor

@shubham1g5 shubham1g5 left a comment

Choose a reason for hiding this comment

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

Does this need to be hotfixed ?

Agree on exploring automated tests with different devices. We do have it already in backlog but have not been able to prioritise.

@ctsims
Copy link
Member Author

ctsims commented May 15, 2023

@shubham1g5 no need for hotfixing. I confirmed in Sumo that there weren't other production projects facing the issue.

@shubham1g5 shubham1g5 merged commit ef45074 into master May 15, 2023
0 of 2 checks passed
@shubham1g5 shubham1g5 deleted the ctsims/grid_image_fix branch May 15, 2023 16:40
@avazirna avazirna added this to the 2.54 milestone Sep 14, 2023
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.

None yet

3 participants