-
Notifications
You must be signed in to change notification settings - Fork 63
Refactor PhotoDetails component to facilitate adding other media types #180
Conversation
This is a great idea. Thanks for doing this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool! Tested locally and it seems to work well 🚀
src/utils/license.js
Outdated
export const checkIsLicense = (licenseOrMark) => { | ||
const licenseOrMarkLower = licenseOrMark ? licenseOrMark.toLowerCase() : '' | ||
return ( | ||
!licenseOrMarkLower.includes('pdm') && !licenseOrMarkLower.includes('cc0') | ||
) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this would be clearer if the function was isPublicDomainLicense
or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something like:
export const isLicense = (licenseOrMark = '') => {
return !['pdm', 'cc0'].includes(licenseOrMark.toLowerCase())
}
export const isPublicDomain = (licenseOrMark = '') => {
return ['pdm', 'cc0'].includes(licenseOrMark.toLowerCase())
}
might be a bit clearer? just one approach, i'm sure there's better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isLicense
is confusing to me, but maybe it's a domain knowledge issue. Isn't CC0
and PDM
still a "license" even it's public domain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't CC0 and PDM still a "license" even it's public domain?
No actually! they are just "marks" or "designations", not licenses in the legal sense. CC even explains this quite poorly, admittedly. That's the purpose of this function, to determine if something is a license or a mark, so that front-end language can be changed.
Boring legal details: https://wiki.creativecommons.org/wiki/CC0_FAQ#What_is_CC0.3F
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh cool, that's good to know! Thanks for the explanation. In that case my comment doesn't really matter 🙂 Maybe we could add a link to that wiki page in the @see
for the function to document for future contributors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a great suggestion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great question @sarayourfriend , and thank you for explanations, @zackkrida !
The CC Website has the following wording for its mission: "Provide Creative Commons licenses and public domain tools ...".
I love the isPublicDomain
function name, but would isPDTool
be more correct in legal terms? I suppose that having a comment about difference between Licenses and Public Domain tools would be enough, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Fixes
Fixes #179 by @obulat
Description
This PR extracts common functionality mainly related to license and attribution from
ImageDetail
components to make them usable with other media types.Technical details
Some functions related to license handling are extracted to
utils/license.js
. It would be nice to find a better home thanutils
in the future.Some spacing and font sizes need to be fixed to match the designs.
Checklist
Update index.md
).main
ormaster
).Developer Certificate of Origin
Developer Certificate of Origin