Skip to content

Simplify the lightbox image caption#1608

Merged
nimmolo merged 4 commits intomainfrom
nimmo-simplify-lightbox-caption
Sep 11, 2023
Merged

Simplify the lightbox image caption#1608
nimmolo merged 4 commits intomainfrom
nimmo-simplify-lightbox-caption

Conversation

@nimmolo
Copy link
Copy Markdown
Contributor

@nimmolo nimmolo commented Sep 11, 2023

To remove extra queries on the obs index.

To reproduce the issue, on <main> go to http://localhost:3000/505457, and then hit "Index" to be sure we're on the same page of the index. Extra queries will fire after the line

  Rendered application/content/_sorter.html.erb (Duration: 0.2ms | Allocations: 172)

If you examine the lightbox caption for observations like 505457 (it's encoded as a data-attribute in the image) you'll find the ID's of some of the associations that are firing the extra queries.

The issue is compounded on the matrix-box carousel PR #1598 because every image in every matrix box gets a lightbox caption.

Note that changes in this PR will also affect lightboxes on the show_obs page. This PR:

  • Replaces the lightbox caption's use of show_obs_title, which has links to owner namings, deprecated names etc., and uses the shorter matrix-box-style obs name instead, linking to the obs.
  • Provisionally eliminates the obs notes from the lightbox caption, because it's the only place i could think that might involve projects. These were some of the extra associations getting loaded. Please let me know if that's not the case. It could be the image copyright instead, for example.

Restored the obs notes because I believe the title was causing all the problems.

To remove extra queries.
@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Sep 11, 2023

Coverage Status

Changes unknown when pulling c4c39b3 on nimmo-simplify-lightbox-caption into ** on main**.

@nimmolo nimmolo marked this pull request as ready for review September 11, 2023 09:13
@nimmolo nimmolo merged commit 259f815 into main Sep 11, 2023
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