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.

Sign up for GitHub

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

Jump to bottom

LIMS-380: Fix position of autoPROC images #831

Merged
merged 3 commits into from
Oct 1, 2024
Merged

LIMS-380: Fix position of autoPROC images #831

merged 3 commits into from
Oct 1, 2024

Conversation

ndg63276
Copy link
Collaborator

JIRA ticket: LIMS-380

Summary:

#390 introduced a bug where image modals opened in autoPROC's summary file opened at the top of the page, requiring much scrolling just to see them.

Changes:

  • Don't set the iframe width/height on load, this confuses the modals
  • Use the jquery on resize event to set the iframe width/height to just smaller than the parent div

To test:

  • Go to a data collection with successful autoPROC runs, eg /dc/visit/cm37235-3/id/14930010
  • Go to the autoPROC tab, click "Logs & Files", then click "View" next to "summary_inlined.html"
  • When the popup loads (can be a few seconds as v large files), scroll down to one of the graphs, and click on it. It should open a larger version, without needing to scroll. Check it closes with the x button.
  • Resize the iframe, check the contents are resized to match the space available, and that images still work
  • Repeat with a "xia2.html" file from a Xia2 run, check the content resizes as the iframe is resized
  • (NB if you try on a txt file, it won't resize as the content is within <pre></pre> tags)

@gfrn
Copy link
Collaborator

gfrn commented Sep 19, 2024 •
edited
Loading

There is a small bug where it doesn't take up 100% of the parent element's width once the dialogue is opened. I believe there is a simpler alternative to JS, which is setting width and height of the iframe to 98%.

It comes with the added benefit of fixing a bug where once the page is resized from mobile size to desktop size, it retains the original size.

I can make a PR for this PR, with the changes made, seems to work with xia2.html and summary_inlined.html, unless there is a known bug with using CSS that way

@ndg63276
Copy link
Collaborator Author

Please, be my guest :)

@ndg63276
Copy link
Collaborator Author

Actually, probably easier for me to add a commit. Is this what you meant? I tried without the onload but without content the sizing was wrong.

@gfrn
Copy link
Collaborator

gfrn commented Sep 20, 2024 •
edited
Loading

I think that's because further down, the default height/width is set to a pixel value on load, I'll create a PR, it may be easier that way

#838

@ndg63276
Copy link
Collaborator Author

Yep that's much neater, thanks!

@ndg63276 ndg63276 changed the base branch from master to pre-release/2024-R4.4 October 1, 2024 09:54
@ndg63276 ndg63276 merged commit c6f264d into pre-release/2024-R4.4 Oct 1, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gfrn gfrn gfrn approved these changes

Assignees
No one assigned
Labels
Ready to Release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ndg63276 @gfrn