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

Implement the revised image permissions statement #775

Closed
6 tasks done
gissoo opened this issue Apr 19, 2022 · 16 comments
Closed
6 tasks done

Implement the revised image permissions statement #775

gissoo opened this issue Apr 19, 2022 · 16 comments
Assignees

Comments

@gissoo
Copy link
Contributor

gissoo commented Apr 19, 2022

Testing notes (qa -- round 2)

  • On the QA site, confirm focus styles are correct for the "Image Permissions Statement" buttons

Testing notes (qa)

  • On the QA site, navigate to some documents with images, and confirm the image permissions statement link and list look correct.

Link to mock-ups on Figma

  • default view (collapsed) with downward phosphor icon chevron – on desktop and mobile
  • selected view (expanded) with upward phosphor icon chevron – on desktop and mobile
  • font styles and alignment for label: link style, below the transcription panel pagination, and bold shelfmark, taking up the full line when expanded – on desktop and mobile
  • spacing between the label and the transcription panel pagination and between each shelfmark when expanded – on desktop and mobile – marked in red on the mock-ups
@gissoo
Copy link
Contributor Author

gissoo commented Apr 20, 2022

@blms @rlskoeser sorry I just added a revised version with the "logos" (really logo/icon placeholders) – everything will work out with the paragraph spacing I had already proposed – the only difference is now there is horizontal spacing and that is 12px (between the logos and the colons) – I am showing two different heights for the icons, 20px (which is matching our S&Co one), and another is 26px, hard to tell what is too large or too small with the appropriate icons – I chose the 595959 color to mute them a bit, and they follow the link style, but I had to add spacing between the icon and the underline and it is 4px. Here is a link to the design on figma

Screen Shot 2022-04-20 at 10.59.21 AM.png

@rlskoeser
Copy link
Contributor

@blms please implement so it supports displaying both attribution and license; we don't currently have both, but it's feasible we will in future

@rlskoeser
Copy link
Contributor

@gissoo the figma link isn't working for me. Could we put unique logos at the bottom instead of displaying per shelfmark?

@gissoo
Copy link
Contributor Author

gissoo commented Apr 20, 2022

@rlskoeser I will email them this week about access and changes in our subscription, this is just unacceptable from them – I thought we agreed to display them this way because Marina mentioned the "shelfmark by shelfmark" and you said it made sense to you – is that not necessary anymore? – I can revise the designs and change the styling if that's the case – I'll create a chore

@gissoo gissoo closed this as completed Apr 20, 2022
@gissoo gissoo reopened this Apr 20, 2022
@rlskoeser
Copy link
Contributor

sorry I didn't think of this before, @gissoo! It seemed reasonable to me to be explicit about rights and permissions when there are multiple shelfmarks, since the attribution and license may differ (probably by instutition/source). It's also a bit more logic for us to figure out how to aggregate them, since the order may not align with the source.

I'm not sure how important it is that the logos be tied to specific shelfmarks — I think it's more important to display them once, and would prefer to display them only once.

@gissoo
Copy link
Contributor Author

gissoo commented Apr 22, 2022

@blms thank you!! The sizes and styles look correct, but some have the image/logo and some like the below don't. Are there cases where there is more than one shelfmark and one logo/right image only? I can't find one to test. If there are no cases like that or you don't want me to test those please feel free to close this PR.

Screen Shot 2022-04-22 at 6 31 07 PM

@gissoo
Copy link
Contributor Author

gissoo commented Apr 22, 2022

@blms just caught this, could you remove the focus indicator on click on Safari and Google Chrome? it's quite distracting – if you can't replicate this please ignore my comment.

Screen Shot 2022-04-22 at 6 44 08 PM

Screen Shot 2022-04-22 at 6 44 40 PM

@blms blms added the ⚠️ tested needs attention Has been through acceptance testing and needs additional work label Apr 25, 2022
@blms
Copy link
Contributor

blms commented Apr 25, 2022

Thanks, I'll work on the focus indicator!

@blms thank you!! The sizes and styles look correct, but some have the image/logo and some like the below don't. Are there cases where there is more than one shelfmark and one logo/right image only? I can't find one to test. If there are no cases like that or you don't want me to test those please feel free to close this PR.

I don't think there are any cases that have both. Also I think it's worth documenting the three different types of thing we might see here:

  • Attribution (text e.g. "Provided by Cambridge University Library"—implemented)
  • License/rights statement (images e.g. "No Known Copyright"—implemented)
  • Logos (images—not yet implemented)

@rlskoeser Does that sound right, and is it indeed true that there aren't any fragments that have both an attribution and a license/rights statement? I didn't find any using this django shell command:

frags = Fragment.objects.filter(manifest__isnull=False)
for f in frags:
    if f.manifest.license and f.attribution:
        print(f.pk)

@rlskoeser
Copy link
Contributor

That's accurate for current status — the CUDL manifests have attribution but no license, and the cached PUL manifests have a license and no attribution.

I talked to Esmé about the PUL manifests for the JTS content because I was surprised they weren't credited anywhere as belonging to JTS; he checked the MOU and they are supposed to be credited, so he added a provenance statement to the metadata, and he also revised the rights statement (it's a CC license now instead of the rightsstatement.org which is their default in figgy). I was planning to handle that in a separate issue as a follow up to this one — does it have a bearing on the design/style questions?

I'm also wondering if we should display the JTS logo when we know content is from JTS, even if it isn't in the manifest...

@blms
Copy link
Contributor

blms commented Apr 25, 2022

I was planning to handle that in a separate issue as a follow up to this one — does it have a bearing on the design/style questions?

@gissoo can answer better, but I imagine it does insofar as the designs need to account for some fragments having both attribution and rights statement. Should these rights statement images be inline with each fragment, or should they be collected at the bottom like the proposed solution for logos? I imagine inline is preferable for that, but not sure.

@rlskoeser
Copy link
Contributor

@gissoo would you prefer we implement all the variants at the same time? I think the updates to the PUL manifest and displaying logos are new features, so we'd want to rewrite the issue as a user story and estimate it (or maybe two issues — one for logos?). I can see the logic for making those changes now. @blms what would be most efficient for you? (we could still merge this PR and do another round of changes for the revisions and related functionality as planned)

@blms
Copy link
Contributor

blms commented Apr 25, 2022

For me it would be easiest to do all at once, but either way is fine as far as I'm concerned. (Also the PR for this issue was already merged, but I need to make the change for the focus indicator and open a new PR!)

@rlskoeser
Copy link
Contributor

Should these rights statement images be inline with each fragment, or should they be collected at the bottom like the proposed solution for logos? I imagine inline is preferable for that, but not sure.

Forgot to respond to this question — in the current design I think rights statements need to be inline so people can tell which rights apply to which shelfmark. Gissoo may revisit this in the new designs, but let's try to keep this simple for now.

blms added a commit that referenced this issue Apr 25, 2022
blms added a commit that referenced this issue Apr 25, 2022
…sions

Permissions statement revisions: focus, fragment view url (#775)
@blms blms added 🗜️ awaiting testing Implemented and ready to be tested and removed ⚠️ tested needs attention Has been through acceptance testing and needs additional work labels Apr 25, 2022
blms added a commit that referenced this issue Apr 25, 2022
@blms blms removed the 🗜️ awaiting testing Implemented and ready to be tested label Apr 25, 2022
blms added a commit that referenced this issue Apr 25, 2022
Apply shelfmark link style to spans in image permissions (#775)
@blms blms added the 🗜️ awaiting testing Implemented and ready to be tested label Apr 26, 2022
@gissoo
Copy link
Contributor Author

gissoo commented Apr 28, 2022

@rlskoeser @blms I think if you could give me some specific requirements for these here or on Slack I can do them quickly and give them to you via some design chores or issues, or we can turn them into separate implementation issues and break this down.

@rlskoeser
Copy link
Contributor

@gissoo I can talk to you about it — LMK what would be easiest for you (can we design for the new version though instead of the interim solution?)

@gissoo
Copy link
Contributor Author

gissoo commented Apr 28, 2022

@rlskoeser @blms your comments make sense. Thank you! I'll make an issue to address this along with the transcription panel issues. Everything else looks fine.

@blms blms closed this as completed Apr 28, 2022
@blms blms removed the 🗜️ awaiting testing Implemented and ready to be tested label Apr 28, 2022
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