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

Entities grid templates #433

Merged
merged 9 commits into from Sep 4, 2019

Conversation

@Atmire-Kristof
Copy link
Contributor

commented Jul 18, 2019

This PR creates grid view templates for all currently available item types, being Journal, Journal Issue, Journal Volume, OrgUnit, Person, Project and Publication.

For all types, the same badge (as was added to search results in list view) is now displayed beneath their thumbnail.

The fields displayed on each card is different depending on the type:

Item Type Top Middle Bottom
Publication dc.title dc.creator, dc.contributor.* dc.description.abstract
Journal dc.title creativework.editor, creativework.publisher dc.description
Journal Issue dc.title creativework.datePublished journal.title
Journal Volume dc.title creativework.datePublished dc.description
OrgUnit organization.legalName organization.foundingDate organization.address.addressCountry, organization.address.addressLocality
Person person.familyName, person.givenName person.email person.jobTitle
Project dc.title dc.description -

Should there be no grid element defined for a type, Publication is used as default.

Copy link
Contributor

left a comment

Thank you @Atmire-Kristof , generally this work it's ok by be. My comments are related with the coherence of the displayed fields list vs grid and the usage of DC and Schema.org fields.

Note: I've just add this issue: "Entities components three structure based on the entity itself" #456
That occurred to me when I started reviewing this PR.

<div class="card-body">
<ds-item-type-badge [object]="object"></ds-item-type-badge>
<ds-truncatable-part [id]="dso.id" [minLines]="3" type="h4">
<h4 class="card-title" [innerHTML]="dso.firstMetadataValue('dc.title')"></h4>

This comment has been minimized.

Copy link
@paulo-graca

paulo-graca Aug 7, 2019

Contributor

The only thing I would like to point out it's the coherence of the presented metadata. The presented data differs from what's displayed on the PersonListElementComponent:
firstMetadataValue('person.familyName') + ', ' + firstMetadataValue('person.givenName')

<ds-truncatable-part [id]="dso.id" [minLines]="3" type="h4">
<h4 class="card-title" [innerHTML]="dso.firstMetadataValue('dc.title')"></h4>
</ds-truncatable-part>
<p *ngIf="dso.hasMetadata('person.identifier.email')" class="item-email card-text text-muted">

This comment has been minimized.

Copy link
@paulo-graca

paulo-graca Aug 7, 2019

Contributor

Can we be "stuck" to DC and Schema.org fields?
person.identifier.email could be replaced with: person.email

<span [innerHTML]="firstMetadataValue('person.identifier.email')"></span>
</ds-truncatable-part>
</p>
<p *ngIf="dso.hasMetadata('person.identifier.jobtitle')" class="item-jobtitle card-text">

This comment has been minimized.

Copy link
@paulo-graca

paulo-graca Aug 7, 2019

Contributor

Can we be "stuck" to DC and Schema.org fields?
person.identifier.jobtitle => person.jobTitle

Copy link
Contributor

left a comment

Request for some missing component comments.

Copy link
Contributor

left a comment

Thank you @Atmire-Kristof for adding these last changes!

Copy link
Member

left a comment

👍 Code looks good, and I gave this a quick test. Seems to work as expected. Thanks @Atmire-Kristof !

@tdonohue tdonohue merged commit 36f134f into DSpace:master Sep 4, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.3%) to 79.388%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.