Skip to content

Conversation

@abhinavohri
Copy link
Contributor

Problem

BB-542 : Create a tab view for entities

Solution

use react bootsrap's tab viewed components to create a clean and organized space for showing entity data.

Screenshots:

Before

Url: https://bookbrainz.org/author/128d9490-ee19-4270-a070-32e0a36847f5

Screencast.from.2025-04-05.15-30-30.webm

After

Desktop View:

Screencast.from.2025-04-05.15-54-14.webm

Mobile View:

Screencast.from.2025-04-05.15-55-21.webm

@abhinavohri
Copy link
Contributor Author

abhinavohri commented Apr 5, 2025

image

Reviews are out of alignment on mobile view. Should I fix it in this PR or in a separate one?

@abhinavohri
Copy link
Contributor Author

image

In the new UI, the release date column goes beyond the width of the rest of components on the page. Should I add a horizontal scroll or is this okay?

@abhinavohri
Copy link
Contributor Author

e626dc4 here I have aligned the heading and the add a review button.

Before:
image

After:
image

@MonkeyDo
Copy link
Member

Hi @KasukabeDefenceForce nice to see you around here again!

Thanks for opening a PR for this, it's a feature I've wanted to see for a long time, and is getting pretty important now with more rich data in BB.

I think we need to discuss the tabs; I think some of the ones you have here totally make sense ("Overview", "Edition", "Collections") while others not as much ("Annnotation", "Add Work/Edition" should probably be in the overview instead).
As a general rule, we want to separate the type of information that is 1. plentiful and 2. most useful to users.

The type of tabs we have might change depending on the entity type.
For Authors, I would definitely want to see Works and Editions (maybe a Translations tab?). There are a lot of these types of relationships and they are the most looked for information (i.e. what are 'the books' by this author?)

@abhinavohri
Copy link
Contributor Author

Hey @MonkeyDo , nice to see you too! Thanks for taking time to review my PR.

Regarding the tabs, should I then filter out the works and translations out of relationships tab and leave relationships tab for less prominent relationships (Like Author-Author, Author-Publisher) ?
Should series be clubbed into the works data or showed separately?

Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

OK, sorry for the delays, it's a very important feature that deserves to be fully thought through, and I ran into some problems while trying to describe my vision.
There is an issue with how we define translations that prevents us from easily separating them from original works. Not exactly sure how to deal with that yet...but I'm open to ideas :)

In the meantime, some review comments below.

id="entity-tabs"
onSelect={handleTabSelect}
>
{tabs.map((tab) => (
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure what the advantage is of defining the tabs beforehand and mapping them here, instead of defining them directly here one after the other.

From my perspective is seems like you need to add !entity.deleted && repeatedly where you could have this condition once before the parent Tabs component (I don;t think we want to show any content for a deleted entity, no wikipedia blurb, no nothing)

I think it would be easier to read the code with the tabs defined in the markup, but maybe I'm missing something? Was there a specific reason you chose this style?

P.S: just to clarify, I'm not saying it's wrong, just that it might be a bit harder to read the component code at a glance.

},
{
content: !entity.deleted &&
<EntityLinks
Copy link
Member

Choose a reason for hiding this comment

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

This was a good way to implement the tab contents!

I have a suggestion for improvements though: we have some table components in the codebase, used for the collections pages, that we can reuse here to have a better tabulated display.

See for example src/client/components/pages/entities/work-table.js
for the Works, which would be used like so:

const worksByAuthor = entityHelper.getRelationshipsByTypeId(entity, [8]).map(rel => rel.target);
[...]
<WorkTable entity={entity} works={worksByAuthor}/>

image

buttonHref={`/work/create?${_kebabCase(entity.type)}=${entity.bbid}`}
entity={entity}
label="Work"
relationshipTypeIds={[8, 1, 31, 62, 34, 35, 37]}
Copy link
Member

Choose a reason for hiding this comment

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

Here's where we get in the thick of it :)

Ideally, I would love to see works divided in separate categories:

  • works written by the author, in their original language
  • translated works (i.e. translations of the original works )
  • works translated by the author (i.e. author is a translator)
  • possibly other categories generated automatically from available rels, or a big "other" category and some additional rel details (linkPhrase) added to the table rows

This will require some heavier processing which I can barely wrap my head around right now.
The issue is for example that translated works have both the original author and the translation author as rels.
So we would need multiple processing steps:

  1. separate works written by the author (relTypeID = 8)
  2. separate the subset of these works that also have a translator rel (relTypeID = 9)

I don't think this is feasible with the available data, as we only have access to the relationships between the original author and the works, and we don't have the other relationships of these works (we would need the "other author translated work" rel for the target work, and we don't).
This looks like a pretty bad failing of the current way we deal with translations...
We could try to fetch extra data for each related work (or each relType 8 target ?) but that seems like a patch.
Not sure what to do here to be honest... for now the easy cop-out is to sort the relType8 works by language and hope for the best? 😭
Any ideas? 🤣

Then comes the decision of how to visually display those categories. I envision different sub-sections in the Works tab which you can switch between using a button/pill.
I'll take for reference what we do on ListenBrainz with tabs + pill buttons:
image

urlPrefix={urlPrefix}
/>,
id: 'translations',
label: 'Translations'
Copy link
Member

Choose a reason for hiding this comment

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

As described above, I think this shouldn't be a separate tab (on the author page at least), but rather a sub-section of the "works" tab.

It will definitely make sense to have a translations tab on the works display page however, when we get to it.

urlPrefix={urlPrefix}
/>,
id: 'relationships',
label: 'Relationships'
Copy link
Member

Choose a reason for hiding this comment

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

For displaying those "other" relationships and avoid having a big ol' messy table, I think they should be displayed in a table but separated by rel type, with a header for each rel type.

We could follow the example set by MusicBrainz:
image
from https://musicbrainz.org/artist/e1d521ea-5b97-4981-987c-ba988b2a87d7/relationships for example

Although note this is for the subset "appearances" relationships, and they also have a big blob of rels above it...

return (
<div>
<h1>
<h1 className="d-flex justify-content-between">
Copy link
Member

Choose a reason for hiding this comment

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

Not so sure about this, it seems to cause an issue with disambiguations:
image

In a flex container, you can add margin-left: auto to the last element (the review button) to automatically align to the right. Maybe that can work here instead of justify-content-between on the parent div?

@ApeKattQuest-MonkeyPython

OK, sorry for the delays, it's a very important feature that deserves to be fully thought through, and I ran into some problems while trying to describe my vision. There is an issue with how we define translations that prevents us from easily separating them from original works. Not exactly sure how to deal with that yet...but I'm open to ideas :)

In the meantime, some review comments below.

the idea I had for translations, is that we'd use "provided story" instead of "wrote" (which is more literary correct (they didn't literally write the translated words in the work after all))
I think that will solve this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants