data citations field authors#629
Conversation
jardakotesovec
left a comment
jardakotesovec
left a comment
There was a problem hiding this comment.
Feel free to reach out on mattermost if something is not clear. Thanks!
| @@ -0,0 +1,136 @@ | |||
| <template> | |||
| <div | |||
| :id="`${props.formId}-${props.name}`" | |||
There was a problem hiding this comment.
Would it make sense to have the label and description - similarly to what we have for ROR?
There was a problem hiding this comment.
Also in this case we need to make it configurable - so it has to be possible to pass it to be able to differentiate between authors and creators
| :id="`${props.formId}-${props.name}`" | ||
| class="pkpFormField pkpFormField--citationAuthors" | ||
| > | ||
| <PkpTable aria-label="CitationAuthors"> |
There was a problem hiding this comment.
This needs to be localised, best I think is just to reference label and description - example you can find in the FieldAffiliations on main - I tweaked that today.
| import TableHeader from '@/components/Table/TableHeader.vue'; | ||
| import TableBody from '@/components/Table/TableBody.vue'; | ||
| import {computed} from 'vue'; | ||
| import {t} from '@/utils/i18n'; |
There was a problem hiding this comment.
Please use useLocalize instead..
import {useLocalize} from '@/composables/useLocalize';
const {t} = useLocalize();
| <a v-if="author.orcid" :href="author.orcid" target="_blank"> | ||
| <Icon | ||
| icon="Orcid" | ||
| :class="'relative top-[-2px] inline-block h-auto w-[16px] align-middle'" |
There was a problem hiding this comment.
I hope this could be vertically centered without using specific -2px offset.. For sizing h-4 w-4 should work.
| </TableCell> | ||
| <TableCell> | ||
| <a | ||
| class="pkpButton flex cursor-pointer items-center border-transparent py-2 text-lg-semibold text-primary hover:enabled:underline" |
There was a problem hiding this comment.
Please use our Button.vue aka PkpButton. It supports all the looks we might need - https://main--6555d3db80418bb1681b8b17.chromatic.com/?path=/story/components-button--link-like-button
| :key="authorIndex" | ||
| > | ||
| <TableCell> | ||
| <FieldText :value="author.givenName" /> |
There was a problem hiding this comment.
I think similarly as you have addAuthor and deleteAuthor - you need to have editAuthor, to correctly update the structure and emit the change.
| <FieldText :value="author.givenName" /> | ||
| </TableCell> | ||
| <TableCell> | ||
| <FieldText v-if="!author.orcid" :value="author.orcid" /> |
There was a problem hiding this comment.
I would suggest just to keep it free text field.. so you don't have to keep additional information to differentiate between fact if it was sourced from somewhere or entered by user.
And than lets have a meeting to figure out how that could be improved.
| <TableRow> | ||
| <TableCell> | ||
| <Button @click="addAuthor()"> | ||
| {{ t('common.add') }} |
There was a problem hiding this comment.
This one would be better to have configurable so it can be passed when this field is created - so we can differentiate between 'Add Author' and 'Add Creator'
There was a problem hiding this comment.
Also add button via #bottom-controls slot: https://github.com/pkp/ui-library/blob/main/src/managers/FileManager/FileManager.vue#L42
Its semantically better for accessibility to have the button below the table, rather than part of it
GaziYucel
force-pushed
the
data-citations-field-authors
branch
2 times, most recently
from
797571a to
f9b7d08
Compare
jardakotesovec
left a comment
jardakotesovec
left a comment
There was a problem hiding this comment.
Just couple more minor things. Thanks!
| default: () => [], | ||
| }, | ||
| /** Default locale of the form */ | ||
| primaryLocale: { |
There was a problem hiding this comment.
I think there is also opportunity to clean up some of the props - which these field does not care about.
I expect these to be - primaryLocale, locales,
| ...row, | ||
| [fieldName]: fieldValue, | ||
| }; | ||
| updatedRow.displayName = ( |
There was a problem hiding this comment.
Why the displayName was introduced? Can't see where thats used.
| </TableRow> | ||
| </TableBody> | ||
| <template #bottom-controls> | ||
| <Button @click="addRow()"> |
There was a problem hiding this comment.
Nothing wrong with using Button like this - but can I ask you to rename it to PkpButton? We do that for couple of exceptions where the name is same as existing html element - like button, table, form. So its more obvious that its our component, and not native html element. Thanks!
GaziYucel
force-pushed
the
data-citations-field-authors
branch
2 times, most recently
from
fe1ff7d to
78a173b
Compare
GaziYucel
force-pushed
the
data-citations-field-authors
branch
from
78a173b to
6afbed1
Compare
2 participants
@jardakotesovec