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

pkp/pkp-lib#1550 Controlled vocabulary support #493

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jyhein
Copy link
Contributor

@jyhein jyhein commented Jan 22, 2025

  • Use external source to add new keywords etc. using plugins
  • Save extra data (identifier, source) with the term to db

@jyhein jyhein force-pushed the f1550 branch 6 times, most recently from a663eaf to 9d70718 Compare January 31, 2025 13:43
Copy link
Member

@touhidurabir touhidurabir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jyhein I added few questions/suggestion, nothing critical . Please take a look when you get time .

Comment on lines +5 to +15
function setSuggestion(value) {
const {name, label = null, identifier = null, ...extraItems} = value;
const suggestion = {
value: value,
label: label ?? name,
identifier: identifier,
...(extraItems ? {extraItems: extraItems} : {}),
};
return suggestion;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason we are making this as general method than as the part of component's method ? Also If I understand correctly, this method takes the data coming from plugin and transform it to support the updated schema . If that is the case , perhaps a different name so that it does not confuse the setSuggestions method coming form FieldBaseAutosuggest component ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm...shouldn't this be private? When it is outside export

Comment on lines 102 to 114
<li v-if="suggestion.identifier?.match(/^http/)">
<a :href="suggestion.identifier" target="_blank" @click.stop>
{{ suggestion.identifier }}
</a>
</li>
<li v-else-if="suggestion.identifier">
{{ suggestion.identifier }}
</li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we merge these 2 together as first we need to check if identifier exists and then if it is an url , then wrap it with anchor tag or just show it as plain text ?


async function metadataDataChange() {
// Some metadata fields need extra data from db not in publication object
fetchForm();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should there be a await here ?

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

Successfully merging this pull request may close these issues.

2 participants