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

feat: Handle VanillaPlus differently in the mod browser #241

Merged
merged 5 commits into from
Aug 2, 2024

Conversation

Jan200101
Copy link
Contributor

VanillaPlus is a Northstar Mod to use more client-sided mods in Vanilla that is installed differently from other mods and is commonly installed by end-users who refuse to read via mod managers.

There are a billion ways to implement this in Viper including but not limited to:

  • catching it during installation and giving a prompt
  • removing install button
  • not listing the mod at all
    and more

I think this is the simplest solution that also doesn't require any translation changes
a separate prompt to inform the user also seems redundant considering its boldly stated in the description
image

@0neGal
Copy link
Owner

0neGal commented Jul 19, 2024

Hmm, never thought to add special conditions for VanillaPlus, but I do agree that it's better with it...

Not entirely convinced on what method would actually be best here, I do think some sort of tooltip or similar would make this better, simply disabling the button may be confusing to some, I'm open to any other suggestions...

@0neGal 0neGal changed the title disable install button for VanillaPlus feat: Handle VanillaPlus differently in the mod browser Jul 19, 2024
@Jan200101
Copy link
Contributor Author

sure
some ideas:

  • disable install button and add a tooltip when hovering over it
  • keep install button enabled but open a prompt stating VanillaPlus needs to be manually installed with a "Guide" button directing the user to the instructions
  • completely hide VanillaPlus, can't cause problems if it ain't there
  • replace "Install" button with a "Guide" button that links to instructions directly

Another idea would be implementing VanillaPlus support directly, but that would need support for Profiles and as far as I heard the idea is for VanillaPlus to turn into a N* fork for better vanilla compatibility, so wasted effort.

@0neGal
Copy link
Owner

0neGal commented Jul 20, 2024

Personally I think the "Guide" button idea is good, would it be best to open the Thunderstore page? Or is there a different page that's preferred for the guide? If so, we could also simply remove both the "Install" and "View" button and replace them with a "Guide" button....

@Jan200101
Copy link
Contributor Author

Jan200101 commented Jul 20, 2024

Personally I think the "Guide" button idea is good, would it be best to open the Thunderstore page? Or is there a different page that's preferred for the guide? If so, we could also simply remove both the "Install" and "View" button and replace them with a "Guide" button....

The Github links to the Thunderstore and the Thunderstore to the github, and both have the same content.
I think linking directly to the README.md would be best to avoid confusion since both Github and Thunderstore display first and foremost.
That way a View button could be kept that links to Thunderstore while a Guide button links to the README

@Jan200101
Copy link
Contributor Author

also, should the change be hardcoded to just VanillaPlus or should it be turned into an object that contains the relevant information?
I'm not aware of any mods that exist or are in the works that would also require this outside of V+, might be good to have regardless.

@0neGal
Copy link
Owner

0neGal commented Jul 20, 2024

The Github links to the Thunderstore and the Thunderstore to the github, and both have the same content.\nI think linking directly to the README.md would be best to avoid confusion since both Github and Thunderstore display first and foremost.\nThat way a View button could be kept that links to Thunderstore while a Guide button links to the README

In that case, I suppose you're right, simply changing "Install" to "Guide" would do it, linking to the GitHub README.md

also, should the change be hardcoded to just VanillaPlus or should it be turned into an object that contains the relevant information?

I would personally add something like this:

let requires_guide = {
    <normalized mod name>: "<link to guide>"
}

Even if currently it's only needed for VanillaPlus... Assuming you're not too familiar with Viper's codebase, by normalized I mean this: mods.normalize(properties.title)

Not sure if requires_guide is the best name, but oh well...

@0neGal 0neGal added the enhancement New feature or request label Jul 20, 2024
@Jan200101
Copy link
Contributor Author

Updated it
image

used the same open icon as the view button
image

added translation for german (also put into the correct position alphabetically to make sure no tooling freaks out

image

Made it display the guide in-app
image

@0neGal
Copy link
Owner

0neGal commented Jul 21, 2024

Nice, however, I do personally think it might be best to simply open it in the default browser, instead of using the mod previewer... And is there a reason we're linking to the blob version of the README.md, instead of simply the repo itself?

@Jan200101
Copy link
Contributor Author

Nice, however, I do personally think it might be best to simply open it in the default browser, instead of using the mod previewer...

Sure, I can amend that

And is there a reason we're linking to the blob version of the README.md, instead of simply the repo itself?

I'm thinking of the end-user here

when opening the repo you just see this
image
how do I install this? This was suppose to be a guide!

linking straight to the readme gets the important bits right up
image

@0neGal
Copy link
Owner

0neGal commented Jul 21, 2024

That's a fair point I didn't really consider, and the blob is better with that in mind...

I suppose we just need the missing localizations then? (@XNovaDelta, @Alystrasz, @KenMizz), and then I'll get this merged soon thereafter...

@Alystrasz
Copy link
Contributor

I suppose we just need the missing localizations then? (@XNovaDelta, @Alystrasz, @KenMizz), and then I'll get this merged soon thereafter...

If I'm not mistaken, the only missing key would be gui.browser.guide, which is the same in French than in English ("Guide").

XNovaDelta added a commit to XNovaDelta/viper that referenced this pull request Jul 22, 2024
@barnabwhy
Copy link

Surely this link would be better for the install guide?

https://github.com/Zayveeo5e/NP.VanillaPlus?tab=readme-ov-file#this-has-a-non-standard-install-process

image

@0neGal
Copy link
Owner

0neGal commented Jul 24, 2024

I think that might also be an idea, although it is possible a browser might not decide to navigate to the right area, or a fullscreen/resize will make it not be scrolled down to the README.md, thoughts @Jan200101?

@Jan200101
Copy link
Contributor Author

idk
I personally don't like relying on anchors because the browser will wait for a contentful paint before setting the viewport correctly, which can be disorienting at times.

the README display on the repo does have the benefit of being smaller, thus easier to parse (similar to Zen or Distractions Free mode in multiple text editors).

but I'm fine with whatever, this is all just personal preference.

@Jan200101
Copy link
Contributor Author

also, one thing I noticed with that anchor link in Firefox is that it ends up being drawn off-screen because Firefox does not take the tab bar into account

@0neGal
Copy link
Owner

0neGal commented Jul 24, 2024

Yeah, so same concerns, I think it's best to just stick with the current link then...

@Jan200101
Copy link
Contributor Author

following up on this, the last thing missing is the chinese translation, correct?

@0neGal
Copy link
Owner

0neGal commented Jul 31, 2024

Correct yeah

@Jan200101
Copy link
Contributor Author

I asked around in some communities and found someone who told me 安装指南 is a translation for "Installation Guide".
Would this be good enough for this case? We could also shorten it to 指南 which would (to my limited research) be as close to the intended meaning of "guide" as possible

@0neGal
Copy link
Owner

0neGal commented Aug 1, 2024

Sounds fine, I suppose when KenMizz sees this they can open a new PR if they disagree! :)

@0neGal
Copy link
Owner

0neGal commented Aug 2, 2024

I'll have this released after #242 is closed, and hopefully together with #239 if I get around to having the time to spend on optimizing and doing a few adjustments.

Unless of course it's very important that this gets released ASAP? Then I'll see to it before then...

@0neGal 0neGal merged commit 5d6ab8e into 0neGal:main Aug 2, 2024
@Jan200101
Copy link
Contributor Author

nah, its not that important

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants