Skip to content

Item Card component displays Critic|Community ratings from setting #6919

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ async function loadDisplaySettings({

const displaySettings = {
customCss: settings.customCss(),
cardRatings: settings.cardRatings() || 'none',
dashboardTheme: settings.dashboardTheme() || 'auto',
dateTimeLocale: settings.dateTimeLocale() || 'auto',
disableCustomCss: Boolean(settings.disableCustomCss()),
Expand Down Expand Up @@ -125,6 +126,7 @@ async function saveDisplaySettings({
userSettings.language(normalizeValue(newDisplaySettings.language));
}
userSettings.customCss(normalizeValue(newDisplaySettings.customCss));
userSettings.cardRatings(newDisplaySettings.cardRatings);
userSettings.dashboardTheme(normalizeValue(newDisplaySettings.dashboardTheme));
userSettings.dateTimeLocale(normalizeValue(newDisplaySettings.dateTimeLocale));
userSettings.disableCustomCss(newDisplaySettings.disableCustomCss);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export interface DisplaySettingsValues {
customCss: string;
cardRatings: string;
dashboardTheme: string;
dateTimeLocale: string;
disableCustomCss: boolean;
Expand Down
49 changes: 49 additions & 0 deletions src/components/cardbuilder/card.scss
Original file line number Diff line number Diff line change
Expand Up @@ -871,3 +871,52 @@ button::-moz-focus-inner {
transform: scale(1.4, 1.4);
transition: 0.2s;
}

.cardRatingContainer {
display: flex;
flex-direction: column;
align-items: flex-start;
position: absolute;
top: 3px;
left: 3px;
padding: 5px;
font-weight: 550;
background-color: rgba(0, 0, 0, 0.5);
border-radius: 15px;
z-index: 10;
}

.card-hoverable:hover .cardRatingContainer {
background-color: rgba(0, 0, 0, 0.0);
}

.cardCriticRating {
background-repeat: no-repeat;
background-size: auto 1.2em;
min-height: 1.2em;
background-position: 2px center;
padding-left: 24px;
}

.cardRatingFresh {
background-image: url(../../assets/img/fresh.svg);
}

.cardRatingRotten {
background-image: url(../../assets/img/rotten.svg);
}

.cardCommunityRating {
min-height: 1.2em;
display: flex;
align-items: center;
margin-right: 3px;
}

.cardStarIcon {
width: auto !important;
height: auto !important;
font-size: 1.4em;
margin-right: 3px;
color: #f2b01e;
}
27 changes: 27 additions & 0 deletions src/components/cardbuilder/cardBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
resolveMixedShapeByAspectRatio
} from './cardBuilderUtils';

import * as userSettings from 'scripts/settings/userSettings';

const enableFocusTransform = !browser.slow && !browser.edge;

/**
Expand Down Expand Up @@ -1127,12 +1129,37 @@
let additionalCardContent = '';

if (layoutManager.desktop && !options.disableHoverMenu) {
additionalCardContent += getRatingHtml(item);
additionalCardContent += getHoverMenuHtml(item, action);
}

return '<' + tagName + ' data-index="' + index + '"' + timerAttributes + actionAttribute + ' data-isfolder="' + (item.IsFolder || false) + '" data-serverid="' + (item.ServerId || options.serverId) + '" data-id="' + (item.Id || item.ItemId) + '" data-type="' + item.Type + '"' + mediaTypeData + collectionTypeData + channelIdData + pathData + positionTicksData + collectionIdData + playlistIdData + contextData + parentIdData + startDate + endDate + ' data-prefix="' + escapeHtml(prefix) + '" class="' + className + '"' + ariaLabelAttribute + '>' + cardImageContainerOpen + innerCardFooter + cardImageContainerClose + overlayButtons + additionalCardContent + cardScalableClose + outerCardFooter + cardBoxClose + '</' + tagName + '>';
}

/**
* Generates HTML markup for card rating.
* @param {object} item - Item used to generate the card rating.
* @returns {string} HTML markup of the card rating.
*/
function getRatingHtml(item) {
const ratingsSetting = userSettings.cardRatings();
if (!ratingsSetting || ratingsSetting === 'none') {
return '';
}

let cardRatingHtml = '';
if (item.CriticRating && ['critic', 'all'].includes(ratingsSetting)) {
const backgroundImageClass = item.CriticRating >= 60 ? 'cardRatingFresh' : 'cardRatingRotten';
cardRatingHtml += `<div class="cardCriticRating ${backgroundImageClass}">${item.CriticRating}</div>`;
}
if (item.CommunityRating && ['community', 'all'].includes(ratingsSetting)) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we can put community ratings first? Same order as in the item details page.

const starIconHtml = '<span class="material-icons cardStarIcon star" aria-hidden="true"></span>'

Check failure on line 1156 in src/components/cardbuilder/cardBuilder.js

View workflow job for this annotation

GitHub Actions / Quality checks 👌🧪 / Run lint 🕵️‍♂️

Missing semicolon
cardRatingHtml += `<div class="cardRating cardCommunityRating">${starIconHtml}${item.CommunityRating.toFixed(1)}</div>`;
}

return cardRatingHtml ? `<div class="cardRatingContainer">${cardRatingHtml}</div>` : '';
}

/**
* Generates HTML markup for the card overlay.
* @param {object} item - Item used to generate the card overlay.
Expand Down
2 changes: 2 additions & 0 deletions src/components/displaySettings/displaySettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ function loadForm(context, user, userSettings) {
context.querySelector('#chkFadein').checked = userSettings.enableFastFadein();
context.querySelector('#chkBlurhash').checked = userSettings.enableBlurhash();
context.querySelector('#chkBackdrops').checked = userSettings.enableBackdrops();
context.querySelector('#selectCardRatings').value = userSettings.cardRatings();
context.querySelector('#chkDetailsBanner').checked = userSettings.detailsBanner();

context.querySelector('#chkDisableCustomCss').checked = userSettings.disableCustomCss();
Expand Down Expand Up @@ -168,6 +169,7 @@ function saveUser(context, user, userSettingsInstance, apiClient) {
userSettingsInstance.enableFastFadein(context.querySelector('#chkFadein').checked);
userSettingsInstance.enableBlurhash(context.querySelector('#chkBlurhash').checked);
userSettingsInstance.enableBackdrops(context.querySelector('#chkBackdrops').checked);
userSettingsInstance.cardRatings(context.querySelector('#selectCardRatings').value);
userSettingsInstance.detailsBanner(context.querySelector('#chkDetailsBanner').checked);

userSettingsInstance.disableCustomCss(context.querySelector('#chkDisableCustomCss').checked);
Expand Down
14 changes: 14 additions & 0 deletions src/components/displaySettings/displaySettings.template.html
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,20 @@ <h2 class="sectionTitle">
<div class="fieldDescription checkboxFieldDescription">${UseEpisodeImagesInNextUpHelp}</div>
</div>

<h2 class="sectionTitle">
${ItemCard}
</h2>
Comment on lines +299 to +301
Copy link
Author

Choose a reason for hiding this comment

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

I did not think any of the other sections fit well enough. Seeing "Item Details" existed I thought "Item Card" was a consistently themed section.
I think "Item Card" is distinct enough and a broad enough design space to justify its own section.


<div class="selectContainer">
<select id="selectCardRatings" is="emby-select" class="selectLayout" label="${LabelCardRatings}">
<option value="none">${None}</option>
<option value="critic">${CriticRating}</option>
<option value="community">${CommunityRating}</option>
<option value="all">${All}</option>
</select>
<div class="fieldDescription">${CardRatingsHelp}</div>
</div>

<h2 class="sectionTitle">
${ItemDetails}
</h2>
Expand Down
20 changes: 17 additions & 3 deletions src/scripts/settings/userSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,9 @@ export class UserSettings {
}

/**
* Get or set customCss.
* @param {string|undefined} [val] - Language.
* @return {string} Language.
* Get or set 'customCss'.
* @param {string|undefined} [val] - 'customCss' state.
* @return {string} 'customCss' state.
*/
customCss(val) {
if (val !== undefined) {
Expand All @@ -321,6 +321,19 @@ export class UserSettings {
return this.get('customCss', false);
}

/**
Copy link
Author

Choose a reason for hiding this comment

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

I know /scripts are marked as deprecated, but from what I can tell updating this file is required when adding a display setting.

I'm open to different approaches but I think its important to have a setting. Both to allow for default behavior of still not showing card ratings, and the extra options I added.

Copy link
Member

Choose a reason for hiding this comment

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

It's adding new files that would be an issue, so it's fine.

* Get or set 'cardRatings' state.
* @param {string|undefined} [val] - 'cardRatings' state.
* @return {string} 'cardRatings' state.
*/
cardRatings(val) {
if (val !== undefined) {
return this.set('cardRatings', val.toString(), false);
}

return this.get('cardRatings', false);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return this.get('cardRatings', false);
return this.get('cardRatings', false) || 'none';

}

/**
* Get or set 'Details Banner' state.
* @param {boolean|undefined} [val] - Flag to enable 'Details Banner' or undefined.
Expand Down Expand Up @@ -694,6 +707,7 @@ export const enableThemeVideos = currentSettings.enableThemeVideos.bind(currentS
export const enableFastFadein = currentSettings.enableFastFadein.bind(currentSettings);
export const enableBlurhash = currentSettings.enableBlurhash.bind(currentSettings);
export const enableBackdrops = currentSettings.enableBackdrops.bind(currentSettings);
export const cardRatings = currentSettings.cardRatings.bind(currentSettings);
export const detailsBanner = currentSettings.detailsBanner.bind(currentSettings);
export const useEpisodeImagesInNextUpAndResume = currentSettings.useEpisodeImagesInNextUpAndResume.bind(currentSettings);
export const language = currentSettings.language.bind(currentSettings);
Expand Down
3 changes: 3 additions & 0 deletions src/strings/en-us.json
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@
"Bwdif": "Bob Weaver DeInterlacing Filter (BWDIF)",
"CancelRecording": "Cancel recording",
"CancelSeries": "Cancel series",
"CardRatingsHelp": "Allows title cards to display critic/community ratings, when available.",
"Casual": "Casual",
"Categories": "Categories",
"ChangingMetadataImageSettingsNewContent": "Changes to metadata or artwork downloading settings will only apply to new content added to your library. To apply the changes to existing titles, you'll need to refresh their metadata manually.",
Expand Down Expand Up @@ -572,6 +573,7 @@
"Inker": "Inker",
"InstallingPackage": "Installing {0} (version {1})",
"InstantMix": "Instant mix",
"ItemCard": "Item Card",
"ItemCount": "{0} items",
"ItemDetails": "Item Details",
"Items": "Items",
Expand Down Expand Up @@ -635,6 +637,7 @@
"LabelCachePath": "Cache path",
"LabelCachePathHelp": "Specify a custom location for server cache files such as images. Leave blank to use the server default.",
"LabelCancelled": "Cancelled",
"LabelCardRatings": "Item Card Ratings Display",
"LabelCertificatePassword": "Certificate password",
"LabelCertificatePasswordHelp": "If your certificate requires a password, please enter it here.",
"LabelChannels": "Channels",
Expand Down
Loading