specify / specify7

Specify 7
https://www.specifysoftware.org/products/specify-7/
GNU General Public License v2.0
60 stars 38 forks source link

Gallery icon remains in fictitious record sets made using "Browse in Forms" #4898

Closed lexiclevenger closed 1 week ago

lexiclevenger commented 1 month ago

Describe the bug The gallery icon was removed from fictitious record sets in the PR #4220. However, it was only removed from fictitious record sets created using the 'Add' button on an existing form, and not from those created using 'Browse in Forms' from a Query.

To Reproduce Steps to reproduce the behavior:

  1. Use a new or existing Query to query for records
  2. Select more than one record
  3. Click 'Browse in Forms' to create a fictitious record set
  4. See gallery icon in the top right

Expected behavior The gallery icon should be removed in order to be consistent with fictitious record sets made using the 'Add' button:

Screenshot 2024-05-08 at 4 53 05 PM

Screenshots

https://github.com/specify/specify7/assets/164079735/23b19d06-b928-4129-bfa7-1a2c1aa424d3

Specify 7 System Information - 2024-05-08T22_03_10.639Z.txt

Please, also fill out the following information manually:

grantfitzsimmons commented 1 month ago

If there are records in a fictitious record set that have attachments, why would we not show the gallery icon? It seems logical to be able to view attachments using the gallery viewer on any record within a fictitious set as long as it is not a new record.

Apologies that I was unable to respond before #4220 was merged. What do you think @CarolineDenis?

grantfitzsimmons commented 1 month ago

Based on our discussion, what we need to do is:

The attachment gallery icon can show when there are no attachments in the record set, but it should only be shown under the conditions above.

maxpatiiuk commented 1 month ago

when all records in a fictitious record set are from the same table

Could you please clarify when is this ever the case? I though Specify only allowed record sets to have records from one table at a time (until we have https://github.com/specify/specify7/issues/2627 😊) is that something unique to fictitious record set? I didn't think so - isn't fictitious record set just a record set we create implicitly on the front-end (and later give user a choice to save it explicitly)?

or is react state getting stale when user switches from one table to another? in that case we should add the key={} prop to clear all states when table name changes