refactor: rework details page and make responsive #616

heyhippari · 2021-01-22T21:23:05Z

Refactors the way we get URLs for details pages, by querying vue-router directly for a name and parameters. This means we're not using straight strings anymore, making future changes easier.
Splits some of the details page to remove complexity in the templates. We currently have:
- artist
- item (generic one)
- musicalbum
- person
- series
Creates a two-column layout component for details pages, making it easier to create new ones
Uses said component on all details pages, to make them responsive
Makes some small adjustments to improve small device versions of pages

locales/en-US.json

mixins/itemHelper.ts

components/Item/Card.vue

components/Players/TrackList.vue

locales/en-US.json

ferferga · 2021-01-26T19:34:10Z

pages/musicalbum/_itemId/index.vue

+      this.setPageTitle({ title: this.item.Name });
+      this.setAppBarOpacity({ opaqueAppBar: false });
+      const hash = this.getBlurhash(this.item, ImageType.Backdrop);
+      this.setBackdrop({ hash });


Hmm, what about setting this in an immediate watcher, so changes of the metadata while in the page are applied (and you also don't need beforeMount anymore)

I moved it to a deep, immediate watcher on all the pages. setAppBarOpacity was moved to created, so that it's earlier in the lifecycle and happens server-side for the SSR version.

pages/series/_itemId/index.vue

locales/en-US.json

ferferga · 2021-02-03T19:33:43Z

pages/artist/_itemId/index.vue

+                      </h2>
+                      <v-col cols="12" sm="9" class="pl-0 pr-0">
+                        <!-- eslint-disable-next-line vue/no-v-html -->
+                        <p class="item-overview" v-html="overview" />


Suggested change

<p class="item-overview" v-html="overview" />

<p class="item-overview" v-text="overview" />

Why not v-text?

Because some providers have markup or paragraphs in them, and these don't work without this.

Imo we should just accept raw text from overviews, but that needs discussion and it's for another PR.

pages/item/_itemId/index.vue

ferferga · 2021-02-03T19:40:13Z

pages/series/_itemId/index.vue

+              v-if="item.Taglines && item.Taglines.length > 0"
+              class="text-subtitle-1 text-truncate"
+            >
+              {{ item.Taglines[0] }}


Why we don't show all of them?

ferferga · 2021-02-03T19:51:36Z

Also, I think content should "stretch" a bit earlier:

ferferga · 2021-02-03T19:55:04Z

There are also some margin issues not present in master:

Master

Your branch (after a rebase against master, just to make sure)

heyhippari · 2021-02-04T10:53:34Z

@ferferga I don't touch that page at all, so I highly doubt it's due to this branch. I've had this same issue before on random commits and have never been able to track it down. It usually gets fixed on another random commit.

I suspect it's the same thing happening with the settings page.

ThibaultNocchi · 2021-02-09T18:30:27Z

@ferferga about this #616 (comment) it's due to how the breakpoints are done. The top picture should be of sm breakpoint I guess, and it goes up to ~900px so it'd be way too large for the mobile view

pages/item/_itemId/index.vue

components/Layout/SwiperSection.vue

components/Layout/ItemCols.vue

codecov-io · 2021-02-09T20:51:53Z

Codecov Report

Merging #616 (de41d6e) into master (cd668fe) will decrease coverage by 0.27%.
The diff coverage is 0.82%.

@@            Coverage Diff             @@
##           master     #616      +/-   ##
==========================================
- Coverage   11.76%   11.48%   -0.28%     
==========================================
  Files         133      135       +2     
  Lines        3451     3543      +92     
  Branches      525      528       +3     
==========================================
+ Hits          406      407       +1     
- Misses       3024     3115      +91     
  Partials       21       21

Impacted Files	Coverage Δ
components/Item/Card/Card.vue	`0.00% <0.00%> (ø)`
components/Item/PersonList.vue	`0.00% <0.00%> (ø)`
components/Item/RelatedItems.vue	`0.00% <0.00%> (ø)`
components/Item/SeasonTabs.vue	`0.00% <0.00%> (ø)`
components/Layout/AudioControls.vue	`0.00% <0.00%> (ø)`
components/Layout/HomeHeader/HomeHeaderItems.vue	`0.00% <ø> (ø)`
components/Layout/SwiperSection.vue	`0.00% <ø> (ø)`
components/Players/TrackList.vue	`0.00% <0.00%> (ø)`
pages/artist/_itemId/index.vue	`0.00% <0.00%> (ø)`
pages/item/_itemId/index.vue	`0.00% <0.00%> (ø)`
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd668fe...de41d6e. Read the comment docs.

mixins/itemHelper.ts

pages/musicalbum/_itemId/index.vue

ThibaultNocchi · 2021-02-10T11:31:05Z

Also, appearances in artist and person pages don't work.

Edit: they work, it's just links in albums or movies to those persons redirect to the item page and not the proper new ones, I'm on it

sonarcloud · 2021-02-10T13:39:47Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

heyhippari mentioned this pull request Jan 23, 2021

First release blockers #557

Closed

16 tasks

ThibaultNocchi reviewed Jan 25, 2021

View reviewed changes

locales/en-US.json Outdated Show resolved Hide resolved

Codex- reviewed Jan 26, 2021

View reviewed changes

mixins/itemHelper.ts Outdated Show resolved Hide resolved

Codex- reviewed Jan 26, 2021

View reviewed changes

mixins/itemHelper.ts Show resolved Hide resolved

heyhippari mentioned this pull request Jan 26, 2021

fix(card hover): fix instantly playing items when clicking in center on touch devices #623

Closed

ferferga requested changes Jan 26, 2021

View reviewed changes

heyhippari force-pushed the details-pages branch from 9a2e6a4 to d1d52e3 Compare January 29, 2021 11:08

heyhippari mentioned this pull request Feb 1, 2021

fix: spacing in artist pages, translation and cards #645

Merged

heyhippari requested a review from ferferga February 3, 2021 16:39

ferferga requested changes Feb 3, 2021

View reviewed changes

ThibaultNocchi force-pushed the details-pages branch 2 times, most recently from 743d467 to 4c47234 Compare February 9, 2021 18:13

ThibaultNocchi force-pushed the details-pages branch from 479e339 to 4c47234 Compare February 9, 2021 19:08

ferferga force-pushed the details-pages branch from 4c47234 to 5027c09 Compare February 9, 2021 20:20

ferferga requested a review from ThibaultNocchi February 9, 2021 20:21

ferferga approved these changes Feb 9, 2021

View reviewed changes

ferferga force-pushed the details-pages branch from 5027c09 to 649c991 Compare February 9, 2021 20:28

ThibaultNocchi requested changes Feb 9, 2021

View reviewed changes

pages/item/_itemId/index.vue Outdated Show resolved Hide resolved

pages/item/_itemId/index.vue Outdated Show resolved Hide resolved

camc314 reviewed Feb 9, 2021

View reviewed changes

components/Layout/SwiperSection.vue Outdated Show resolved Hide resolved

components/Layout/ItemCols.vue Show resolved Hide resolved

ferferga force-pushed the details-pages branch from 649c991 to 40dc356 Compare February 9, 2021 20:49

ferferga force-pushed the details-pages branch from 40dc356 to 1f7eac6 Compare February 9, 2021 20:51

ferferga requested a review from ThibaultNocchi February 9, 2021 20:53

camc314 force-pushed the details-pages branch 2 times, most recently from f0c42f8 to 6f81e22 Compare February 9, 2021 21:20

Codex- reviewed Feb 9, 2021

View reviewed changes

mixins/itemHelper.ts Show resolved Hide resolved

ThibaultNocchi requested changes Feb 10, 2021

View reviewed changes

pages/musicalbum/_itemId/index.vue Outdated Show resolved Hide resolved

ThibaultNocchi force-pushed the details-pages branch from de41d6e to 0327c37 Compare February 10, 2021 11:30

ferferga requested a review from ThibaultNocchi February 10, 2021 13:07

heyhippari and others added 4 commits February 10, 2021 14:07

refactor: rework details page and make responsive

da7f899

refactor(navigation): refactor navigation to use a mixin to get links

cf170df

refactor(swiper section): use smaller font on sm devices

4926fba

fix(musicalbumindex): fix link to artist

86a9f3e

ferferga force-pushed the details-pages branch from 29abf5d to 1a9345c Compare February 10, 2021 13:08

refactor: cleanup classes and fix link

44450c2

ferferga force-pushed the details-pages branch from 1a9345c to 44450c2 Compare February 10, 2021 13:12

fix(itemhelper): add possibility to override item type when using helper

332f693

ThibaultNocchi force-pushed the details-pages branch from 3e9ff2b to 332f693 Compare February 10, 2021 13:38

ThibaultNocchi approved these changes Feb 10, 2021

View reviewed changes

ferferga merged commit 17ac05f into master Feb 10, 2021

ferferga deleted the details-pages branch February 10, 2021 13:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: rework details page and make responsive #616

refactor: rework details page and make responsive #616

heyhippari commented Jan 22, 2021

ferferga Jan 26, 2021

heyhippari Jan 29, 2021

ferferga Feb 3, 2021

heyhippari Feb 3, 2021

ferferga Feb 3, 2021

ferferga Feb 3, 2021

ferferga commented Feb 3, 2021

ferferga commented Feb 3, 2021

heyhippari commented Feb 4, 2021 •

edited

Loading

ThibaultNocchi commented Feb 9, 2021

codecov-io commented Feb 9, 2021 •

edited

Loading

ThibaultNocchi commented Feb 10, 2021 •

edited

Loading

sonarcloud bot commented Feb 10, 2021

	<p class="item-overview" v-html="overview" />
	<p class="item-overview" v-text="overview" />

refactor: rework details page and make responsive #616

refactor: rework details page and make responsive #616

Conversation

heyhippari commented Jan 22, 2021

ferferga Jan 26, 2021

Choose a reason for hiding this comment

heyhippari Jan 29, 2021

Choose a reason for hiding this comment

ferferga Feb 3, 2021

Choose a reason for hiding this comment

heyhippari Feb 3, 2021

Choose a reason for hiding this comment

ferferga Feb 3, 2021

Choose a reason for hiding this comment

ferferga Feb 3, 2021

Choose a reason for hiding this comment

ferferga commented Feb 3, 2021

ferferga commented Feb 3, 2021

heyhippari commented Feb 4, 2021 • edited Loading

ThibaultNocchi commented Feb 9, 2021

codecov-io commented Feb 9, 2021 • edited Loading

Codecov Report

ThibaultNocchi commented Feb 10, 2021 • edited Loading

sonarcloud bot commented Feb 10, 2021

heyhippari commented Feb 4, 2021 •

edited

Loading

codecov-io commented Feb 9, 2021 •

edited

Loading

ThibaultNocchi commented Feb 10, 2021 •

edited

Loading