Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mod preview panel #1680

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open

Mod preview panel #1680

wants to merge 16 commits into from

Conversation

ebkr
Copy link
Owner

@ebkr ebkr commented Mar 23, 2025

No description provided.

ebkr added 14 commits March 22, 2025 10:19
- Fixed mod list refresh issue on re-open

- Prevented image links opening via internal browser

- Incorrect README and CHANGELOG can no longer appear if loaded after a new selected README/CHANGELOG has been loaded.
- Switched implementation to rely on provider binding

- Changed "Rating" to "Likes"

- Removed Donate button from Online preview

- Summary now has a border

- Download mod modal now opens
@ebkr ebkr requested a review from anttimaki March 23, 2025 00:12
@@ -1,6 +1,8 @@
<template>
<div>
<router-view v-if="visible"/>
<div class="router-view">
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do these wrappers (there's another in Manager.vue) do? I couldn't find any styles targeting the class name.

@@ -0,0 +1,75 @@
<script setup lang="ts">
Copy link
Collaborator

Choose a reason for hiding this comment

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

On TSMM, it seems the button that should open this modal does nothing. If the modal is made visible via devtools, changing the values in dropdown don't affect the ordering. IDK why, didn't look into it yet.

const directions = Object.keys(SortDirection);
const behaviours = Object.keys(SortingStyle);

const selectedBehaviour = computed(() => store.state.modFilters.sortBehaviour);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I noticed the computed accepts a setter, e.g. something like this:

const selectedBehaviour = computed({
    get() {
        return store.state.modFilters.sortBehaviour;
    },
    set(value: string) {
        // Note that setSortBehaviour currently expects object key and we pass the value directly.
        store.commit("modFilters/setSortBehaviour", value);
    }
});

Markup could then use v-model:

<select class="select select--content-spacing" v-model="selectedBehaviour">
    <option v-for="(value, key) in SortingStyle" :value="value" :key="`sort-behaviour--${key}`">
        {{ value }}
    </option>
</select>

This would remove the need to swap between the key and the value.

@@ -0,0 +1,99 @@
<script lang="ts" setup>
import { computed } from 'vue';
import markdownit from 'markdown-it';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not 100% certain how experimental this is, but the new website uses an undocumented endpoint, e.g. https://thunderstore.io/api/cyberstorm/package/ymc_mhz/morehead/latest/readme/ (also available for changelog). The major thing is it returns HTML instead of MD, so we could avoid requiring these dependencies, and IIRC get some security against XSS at the same time. Additionally it would make it easier to have the content look the same on the manager and the website, so the author doesn't need to specifically target one or another.

Also Oksamies might be able to provide some CSS that would make it even easier to get the content look the same, but I'm not 100% sure how feasible that is.

import { computed } from 'vue';
import markdownit from 'markdown-it';
import { markdownItTable } from 'markdown-it-table';
import LinkProvider from 'src/providers/components/LinkProvider';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Non-relative import. There's more of these in other files.

class="input"
type="text"
placeholder="Search for a mod"
autocomplete="off"
Copy link
Collaborator

Choose a reason for hiding this comment

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

This shouldn't be removed, it's needed in TSMM.

<div class="input-group">
<div class="input-group input-group--flex">
<label for="thunderstore-category-filter">&nbsp;</label>
<button
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like how using the modal button here instead of the selects clean up the UI 👍Outside of the scope here but we should consider something similar to installed mod view?

Comment on lines 115 to 116
sortingDirectionModel = SortingDirection.STANDARD;
sortingStyleModel = SortingStyle.DEFAULT;
Copy link
Collaborator

Choose a reason for hiding this comment

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

These could be cleaned up now.

@@ -226,8 +234,59 @@ export default class OnlineModView extends Vue {
});
}

openPreviewForMod(mod: ThunderstoreMod) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nitpick: really a "toggle" rather than "open" function.

},

setSortDirection(state: State, direction: keyof typeof SortDirection) {
console.log(direction)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Leftover

@anttimaki
Copy link
Collaborator

Some color-related readability issues especially on dark mode:
image

Tables, at least inside a collapsible element, don't render contents:
image

Both examples are from https://new.thunderstore.io/c/repo/p/YMC_MHZ/MoreHead/

@anttimaki
Copy link
Collaborator

Not sure what part of the changes, but TSMM now has two scroll bars, making things awkward. Also don't know if it's easier to address this on TSMM side or R2MM.

image

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.

2 participants