Skip to content

ContentDB: Add reviews tab#15254

Merged
rubenwardy merged 5 commits into
luanti-org:masterfrom
rubenwardy:contentdb-reviews
Apr 13, 2025
Merged

ContentDB: Add reviews tab#15254
rubenwardy merged 5 commits into
luanti-org:masterfrom
rubenwardy:contentdb-reviews

Conversation

@rubenwardy

@rubenwardy rubenwardy commented Oct 8, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

I'm not really happy with this design but I'm limited by formspecs and hypertext.

The reviews are rendered as a single hypertext element, with the hypertext generated on the server. Whilst it would have been nice to make it a bit prettier with a scroll container and better placement, putting hypertext in a scroll container results in a double vertical scroll which is a pain to use

image

To do

  • Showing network errors
  • Move "Leave a review..." button?

How to test

Use the package dialog, including existing description and info tabs and the new reviews tab

@rubenwardy rubenwardy added Feature ✨ PRs that add or enhance a feature @ Content / PkgMgr labels Oct 8, 2024
@rubenwardy
rubenwardy requested a review from grorp October 8, 2024 12:27
@rubenwardy
rubenwardy marked this pull request as draft October 8, 2024 12:35
@rubenwardy
rubenwardy marked this pull request as ready for review October 8, 2024 12:39
@rubenwardy rubenwardy mentioned this pull request Oct 8, 2024
Open
8 tasks
@Zughy Zughy added Roadmap The change matches an item on the current roadmap Roadmap: supported by core dev PR not adhering to the roadmap, yet some core dev decided to take care of it and removed Roadmap The change matches an item on the current roadmap labels Oct 8, 2024
grorp
grorp reviewed Oct 11, 2024
View reviewed changes

@grorp grorp left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm missing some information:

  • Upvote/downvote count of reviews (to judge the quality of reviews)

  • Number of comments

    Before I click the "Comments" link, I want to know if there are even any comments. If there are very many comments, I will be more interested to have a look at them.

EDIT: A review count in the "Reviews" tabheader might be nice too

Comment thread LICENSE.txt
Comment thread builtin/mainmenu/content/dlg_package.lua
Comment thread builtin/mainmenu/content/dlg_package.lua Outdated
@grorp grorp added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Oct 11, 2024
grorp referenced this pull request in luanti-org/contentdb Oct 11, 2024
@rubenwardy
Review hypertext: Fix links in reviews not working
52df207
Athozus
Athozus reviewed Oct 25, 2024
View reviewed changes
Comment thread textures/base/pack/contentdb_neutral.png
@rubenwardy rubenwardy added Roadmap The change matches an item on the current roadmap and removed Roadmap: supported by core dev PR not adhering to the roadmap, yet some core dev decided to take care of it labels Nov 3, 2024
@rubenwardy
rubenwardy force-pushed the contentdb-reviews branch 2 times, most recently from 60af18d to c0c66ad Compare November 15, 2024 18:51
@rubenwardy

rubenwardy commented Nov 15, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

Updated

image

@grorp

grorp commented Nov 16, 2024

Copy link
Copy Markdown
Member

"Comments [10] - 17 helpful, 1 unhelpful" parses as "Comments [10] - 17 helpful comments, 1 unhelpful comment" for me. Perhaps this could be fixed by reversing the order.

@rubenwardy rubenwardy added Action / change needed Code still needs changes (PR) / more information requested (Issues) and removed Action / change needed Code still needs changes (PR) / more information requested (Issues) labels Dec 24, 2024
@rubenwardy

rubenwardy commented Dec 24, 2024

Copy link
Copy Markdown
Member Author

image

@rubenwardy rubenwardy removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Dec 24, 2024
@grorp
grorp self-requested a review February 28, 2025 03:16
@grorp grorp mentioned this pull request Mar 1, 2025
Closed
grorp
grorp approved these changes Mar 1, 2025
View reviewed changes

@grorp grorp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good.

As suggested, I have opened a separate issue for the Apache 2.0 license compat question: #15857

SmallJoker
SmallJoker reviewed Mar 16, 2025
View reviewed changes

@SmallJoker SmallJoker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Works well so far. I've got a few questions/remarks...

Comment thread builtin/async/mainmenu.lua Outdated
Comment thread builtin/mainmenu/content/dlg_package.lua
Comment thread builtin/mainmenu/content/dlg_package.lua
@ghost

ghost commented Mar 25, 2025

Copy link
Copy Markdown

Can this not show amount of comments, or at least have a settings option to choose show or not show amount of comments in Reviews[count] tab? I do not want to load reviews before I click the tab

@rubenwardy

rubenwardy commented Mar 25, 2025

Copy link
Copy Markdown
Member Author

Can this not show amount of comments, or at least have a settings option to choose show or not show amount of comments in Reviews[count] tab? I do not want to load reviews before I click the tab

For performance reasons, the reviews aren't loaded until you click the reviews tab. The count comes from the "+4 / 3 / -3" shown at the top of the screen

By comments, do you mean reviews? Is the user need that you don't want to feel compelled to open the reviews tab, like it's an unread notification?

@SmallJoker
SmallJoker self-requested a review April 6, 2025 15:41
SmallJoker
SmallJoker approved these changes Apr 12, 2025
View reviewed changes

@SmallJoker SmallJoker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code looks good and works. Added the URL to have a reference in the code.

@rubenwardy
rubenwardy merged commit 75862e3 into luanti-org:master Apr 13, 2025
@rubenwardy
rubenwardy deleted the contentdb-reviews branch April 13, 2025 15:07
mjz19910 pushed a commit to mjz19910/luanti that referenced this pull request Apr 18, 2025
@rubenwardy @mjz19910
ContentDB: Add reviews tab (luanti-org#15254)
2cdca74
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SmallJoker SmallJoker SmallJoker approved these changes
@grorp grorp grorp approved these changes
+1 more reviewer
@Athozus Athozus Athozus left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

@ Content / PkgMgr Feature ✨ PRs that add or enhance a feature @ Mainmenu / Settingsmenu Roadmap The change matches an item on the current roadmap >= Two approvals ✅ ✅

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@rubenwardy @grorp @SmallJoker @Athozus @Zughy