Improved list view for scenes, galleries and performers

[cj12312021]

This pull request improves the list, which was previously unusable, in my opinion. The big addition is the ability to show or hide specific columns. With that capability, I will add more columns to the existing table. Aside from that, I touched up a few areas to modernize the table's appearance.

Here is an image of the updated table:

The selectable columns can be seen here:

Resolves #4165

[DogmaDragon]

Welcome changes, my issue is that it fractures the UI even more than it already is by only addressing the scenes section.

Also, being able to change the order in which columns are presented would be ideal, as the fields you want to see can depend on preference or workflow.

[cj12312021]

The style is inherited by other pages that use a table for lists. Those tables would just be missing the column selector. Given the work involved, it wouldn't make sense to do everything at once.

And yes, rearrangeable columns would be nice but is not a requirement. I can’t commit to that work, but it can be added later.

[InfiniteStash]

Looks great, but I agree with @DogmaDragon that it's not ideal to have this change only for scenes. I'm happy to take a stab at creating an abstraction that can be reused for other pages if you'd like.

[cj12312021]

Sure, I won’t turn down help that would make migrating the work easier. ATM it’s not clear to me how much further abstraction is possible here to meaningfully help migration. I would think each new column for other pages would still require their own function. The list pages for galleries, performers, and
tags(this one currently does its own thing) are bare and would require more columns.

This work is just more digestible for me, going page by page to make managing my time easier. I have a habit of doing very long sprints when I tackle work, which has a negative impact on my eating and sleeping habits. I end up not doing much of either, as I have a hard time getting the work out of my mind.

Update:
Allowing a review on one page first also helps solidify and minimize changes that would need to be made for subsequent reviews on the other pages. Making other pages quicker to put out.

[InfiniteStash]

@cj12312021 Hope you don't mind I pushed a commit with some refactoring. It behaves exactly the same, but I've abstracted out the column selection so it'll be easier to reuse elsewhere. Hopefully someone else will pick up the other tables once this PR has landed.

[cj12312021]

Much appreciated, Infinite. I'll go through your changes and see what I can learn.
I plan on continuing the work to the other pages after this pull request is wrapped up. I'll just be doing it a page at a time. The performer's page is my next target.

[cj12312021]

@InfiniteStash, I think you forgot to commit changes to config.ts. It looks defaultSceneColumns in the config should have changes to tableColumns, which would map the table name string to IColumn[]. I'm not sure if there are other changes you meant to include in that file.

[InfiniteStash]

@InfiniteStash, I think you forgot to commit changes to config.ts. It looks defaultSceneColumns in the config should have changes to tableColumns, which would map the table name string to IColumn[]. I'm not sure if there are other changes you meant to include in that file.

I forgot about that entirely. It should be tableColumns: Record<string, string[]>. The way the config is loaded the types aren't actually used, which is why I forgot about it. Something for a different PR.

[cj12312021]

Perfect thanks. I just pushed the update.

[cj12312021]

The way the config is loaded the types aren't actually used, which is why I forgot about it. Something for a different PR.

This part of your comment just clicked. I see. The runtime issue I was running into when I pulled the changes was due to the code not accounting for tableColumns being undefined during the first run. I went ahead and removed the code in config.ts and updated the column selector code to consider that tableColumns could be undefined.

[cj12312021]

I had a bit more free time than expected, so I went ahead and included the performer's page updates in the pull request:

Here are the available columns:

I will hold off on making any more page additions until after this pull request goes in.

[WithoutPants]

I've added a generic ListTable component and converted the SceneListTable and PerformerListTable components to use the new component, along with a set of general refactors. The branch is here. I haven't pushed it to this branch because it's a significant change, and you should get the opportunity to review it. Let me know. If this looks good, I'll also do the GalleryListTable and this should be ready to merge.

[cj12312021]

Thanks for going through this work. I've taken a quick look at the change to get a general understanding of the restructure—good touch with the mandatory column change. I also built and ran the change to ensure no new issues exist. Feel free to merge your branch and this branch when you're ready.

[cj12312021]

After your merge, I noticed the unique hover solution I created for lists with anchor items was being used on all cells with ellipsed data, which was not the intended use case. Ellipsed text not in a list should just use the title property. Using that style on all ellipsed cells breaks some cells. My latest push addresses that issue.

[WithoutPants]

This was in response to this commit. I was trying to avoid ignoring the linter rules and misunderstood the intention. Having taken another look, this rule only (td li a .ellips-data:hover) applies within a comma-list (which only appears to be MovieCell in SceneListTable and SceneCell in my added GalleryListTable). Therefore, the rule should be able to be changed to .comma-list li .ellips-data:hover. Likewise .table-list td a [div/span] can be simplified to .table-list td a to avoid ignoring linter rules. I'll make this change. Any further refinements can go in another PR.

[WithoutPants]

For future refinement ideas, I think we should utilise the vertical space afforded by the cover image to show lists vertically rather than horizontally. Note also that if Galleries is last cell in the scene table, hovering over a value won't show the full title since it overflows off the screen.

[cj12312021]

This was in response to this commit. I was trying to avoid ignoring the linter rules and misunderstood the intention

Ah okay. Yeah, I could have saved you some trouble there by removing the a in the selector. I tried testing without the anchor in the selector last night, but things didn't work as expected. It was a late night so I likely forgot a step in the build process.

[cj12312021]

Note also that if Galleries is last cell in the scene table, hovering over a value won't show the full title since it overflows off the screen.

Noted. Removing the position: absolute rule would resolve the issue, but it would also cause the table contents to shift every time an ellipsed cell is hovering. Another solution may be to keep the max-width of the hovered text box set to the width of the cell and instead let the content break vertically. But then you would end up with the same issue with hovered text at the bottom of the table. An improved solution will require more thought. There may not be a compromise-free solution.