chore: UI fixes/additions #1563
Conversation
ydkmlt84
commented
Feb 9, 2025
ydkmlt84
commented
- added library name to media card on hover
- made transparent background on mediacard hover a little darker
- fixed MediaModal popup during removal of a global exclusionn
- fixed truncate/scroll issue with description on collection page
- other various small fixes/changes
- made transparent background on mediacard hover a little darker - fixed MediaModal popup during removal of a global exclusionn - fixed truncate/scroll issue with description on collection page - other various small fixes/changes
|
Fixes #1432 |
| useEffect(() => { | ||
| GetApiHandler('/plex/libraries').then( | ||
| (libraries: { key: string; title: string }[]) => { | ||
| if (!libraries || !Array.isArray(libraries)) return | ||
|
|
||
| // Convert librarySectionID (key) to integer before storing it | ||
| const libraryMapping = libraries.reduce<Record<number, string>>( | ||
| (acc, library) => { | ||
| acc[parseInt(library.key, 10)] = library.title | ||
| return acc | ||
| }, | ||
| {}, | ||
| ) | ||
|
|
||
| setLibraryMap(libraryMapping) | ||
| }, | ||
| ) | ||
| }, []) |
There was a problem hiding this comment.
This creates an API call for each media item which is not the best. You can move it to the parent and pass the libraries down instead. There are alternatives to axios (what the handlers use under the hood) which can de-dupe & cache requests, such as SWR and React Query, so you don't have to think about it too much. We've got quite a few places that do the same fetch and store state like this, so we ought to move them to that or create hooks to keep things tidy at some point.
| const libraryTitle = React.useMemo(() => { | ||
| if (!isLibraryMapReady || typeof libraryId !== 'number') return '' | ||
| return libraryMap[libraryId] ?? 'Unknown Library' | ||
| }, [isLibraryMapReady, libraryId, libraryMap]) |
There was a problem hiding this comment.
There is no need for useMemo here as the logic is not complex:
| const libraryTitle = React.useMemo(() => { | |
| if (!isLibraryMapReady || typeof libraryId !== 'number') return '' | |
| return libraryMap[libraryId] ?? 'Unknown Library' | |
| }, [isLibraryMapReady, libraryId, libraryMap]) | |
| const libraryTitle = | |
| !isLibraryMapReady || typeof libraryId !== 'number' | |
| ? '' | |
| : (libraryMap[libraryId] ?? 'Unknown Library') |
Or better yet, it looks like libraryId will never be undefined looking at the call site. So you can make libraryId just a number and remove that condition.
Though now I wish I didn't start looking into libraryId. Plex returns it as a string but we appear to cast it to a number in a lot of places which seems a bit pointless.
| className="mt-1 flex h-4 text-xs" | ||
| style={{ | ||
| WebkitLineClamp: 5, | ||
| display: '-webkit-box', | ||
| overflow: 'hidden', | ||
| WebkitBoxOrient: 'vertical', | ||
| wordBreak: 'break-word', |
There was a problem hiding this comment.
I'm not entirely sure what these inline styles were supposed to be doing when there wasn't any content inside, I presume at some point there might have been 🤷
Library names get cut off when they're too long, which isn't too hard to do given there isn't a lot of space to work with. I would suggest removing the fixed height and all of the inline styles as we don't need to clamp. If you want to limit the display to say 3 lines, then use line-clamp-3 and remove flex as it's not doing anything here.
| <span className="whitespace-normal text-white"> | ||
| <span className="font-semibold">Library: </span> | ||
| {libraryTitle} | ||
| </span> |
There was a problem hiding this comment.
The surrounding span isn't doing anything here:
| <span className="whitespace-normal text-white"> | |
| <span className="font-semibold">Library: </span> | |
| {libraryTitle} | |
| </span> | |
| <> | |
| <span className="font-semibold">Library: </span> | |
| {libraryTitle} | |
| </> |
| @@ -138,7 +139,7 @@ const OverviewContent = (props: IOverviewContent) => { | |||
| <li key={+el.ratingKey}> | |||
| <MediaCard | |||
| id={+el.ratingKey} | |||
| libraryId={props.libraryId} | |||
| libraryId={+el.librarySectionID || props.libraryId} | |||
There was a problem hiding this comment.
How come you needed to do this? From my testing all 3 places this component is used populates the libraryId.
| @@ -35,7 +35,6 @@ const CollectionItem = (props: ICollectionItem) => { | |||
| src={`https://image.tmdb.org/t/p/w500/${props.collection.media[1].image_path}`} | |||
| alt="img" | |||
| /> | |||
| <div className="collection-backdrop"></div> | |||
There was a problem hiding this comment.
You can remove the class definition from globals.css now
| apply the change to all collections | ||
| </p> | ||
| </Modal> | ||
| <div onClick={(e) => e.stopPropagation()}> |
There was a problem hiding this comment.
This wrapping div is not necessary. Update handle() to call e.stopPropagation() like handlePopup() does.
