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

As a user, I want the full citation for a transcription in context so I know who authored it and where it came from. #959

Closed
2 tasks done
rlskoeser opened this issue Jul 18, 2022 · 5 comments
Assignees

Comments

@rlskoeser
Copy link
Contributor

rlskoeser commented Jul 18, 2022

testing notes (qa)

  • Check the transcriptions for a few different kinds of documents—example PGPIDs could be: 606, 9121, 2691, 4508, 3254—and confirm that the citation in gray text provides the necessary information, including when switching between multiple transcriptions.

known revisions needed

  • switching back to Gil edition on pgpid 606, Goitein on pgpid 9121 loses some info (related to links?)

dev notes

Transcription citation should be displayed in gray at the top of the transcription panel.

This was already implemented in an earlier phase even though it was not mvp, but I'm not sure it was tested directly and I noticed in passing some citations that didn't look correct. Let's check the dev implementation and be sure we're using the correct citation and then test it with all source variations.

@blms
Copy link
Contributor

blms commented Jul 25, 2022

We are indeed using the same short citation format that is used in the metadata section. A few things I noticed:

  1. That citation format doesn't seem to italicize book titles—I recall vaguely that it was intended to be plaintext, but should it be?

  2. Should the citation format include "[digital geniza document edition]"? Right now we're only including that on the long format, but in the designs it appears next to the transcriptions.

  3. I noticed sometimes the spacing is different depending on whether or not there is a heading. It interacts strangely with the min-height we set on the citation. I think that's more of a problem with the transcription styling than the citation styling, though. See below:

    Spacing differences Screen Shot 2022-07-25 at 2 18 49 PM Screen Shot 2022-07-25 at 2 18 53 PM Screen Shot 2022-07-25 at 2 18 57 PM
  4. Unrelated, but I found during this testing that the transcription switcher doesn't work at all for documents with no images, so time to fix that!

@rlskoeser
Copy link
Contributor Author

@blms I think we should use the formatted citation. Maybe it's easier (except for layout/spacing) if we use the formatted version — the formatted_display method. We've discussed including the citation in the copy/paste version of the transcription text, and in that case researchers would definitely want the full citation — and what they copy should probably match what we display. Ergo, full citation with formatting please.

I think the min-height really only comes into play with documents that have more than one transcription. Maybe that helps us somehow with how we approach it?

@blms blms self-assigned this Jul 25, 2022
blms added a commit that referenced this issue Jul 25, 2022
rlskoeser added a commit that referenced this issue Jul 26, 2022
…switcher-revision

Fix transcription switcher on docs w/ no images (#899), use full citation (#959)
@blms blms added the 🗜️ awaiting testing Implemented and ready to be tested label Jul 26, 2022
@richmanrachel richmanrachel removed the 🗜️ awaiting testing Implemented and ready to be tested label Jul 26, 2022
@richmanrachel
Copy link

Love it! Closing :)

@blms
Copy link
Contributor

blms commented Jul 26, 2022

Reopening due to the issue I found with links, marking as tested needs attention too

@blms blms reopened this Jul 26, 2022
@blms blms added the ⚠️ tested needs attention Has been through acceptance testing and needs additional work label Jul 26, 2022
blms added a commit that referenced this issue Jul 26, 2022
@blms blms removed the ⚠️ tested needs attention Has been through acceptance testing and needs additional work label Jul 26, 2022
@rlskoeser
Copy link
Contributor Author

tested switching between editions on pgpid 606 and pgpid 9121, I think it's behaving as expected; links are preserved and display properly after switching

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

No branches or pull requests

3 participants