-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: update item page design w/ data provider section #1912
feat: update item page design w/ data provider section #1912
Conversation
JIRA ticket: EC-6178 |
Preview deployment: https://pr-1912.portal-js.dev.eanadev.org/ |
namePrefLanguage() { | ||
return this.getPrefLanguage('edmDataProvider', { def: [{ prefLabel: this.dataProvider }] }); | ||
}, | ||
nativeName() { |
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.
Is this really the native name if there is no entity?
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.
It depends on the data from the record.
Changed the variable name to reflect this.
However I wonder if we need this at all.
If not it would also mean we don't need the getPrefLanguage method in a mixin, which would address your other comment regarding moving the prop/parameter.
if (['edmDataProvider', 'edmProvider'].includes(fieldName)) { | ||
nativeLocale = this.getNativeLocale(fieldData); | ||
} | ||
return nativeLocale || this.metadataLanguage || this.$i18n.locale; |
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 don't think we should refer to this.metadataLanguage
here without any context of where it's come from.
Either:
- Move the
metadataLanguage
prop into the mixin; or - Make
metadataLanguage
a parameter of the function
I'd suggest option 2.
:items="items" | ||
view="explore" | ||
class="mb-0" | ||
data-qa="similar items" | ||
/> | ||
<b-button |
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 should be a link.
variant="outline-secondary" | ||
@click="keycloakLogin" | ||
> | ||
{{ $t('related.items.loginForMore', { other: items.length ? 'other' : ''}) }} |
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 not going to translate properly.
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.
And space is needed before one of the }
Kudos, SonarCloud Quality Gate passed! |
No description provided.