[feat] Add file sizes+oversize warning #791

TheUnifox · 2025-02-07T07:44:08Z

Scope

This PR is to prepare for a backend change adding file sizes to mod versions. It also adds an oversize warning when a mod is over 50MB, and further warning if over 100MB. It's rare that a mod is that large, but in case a large mod is uploaded, the user should be warned if a mod is overly large.

This should be able to be pulled without anything breaking before the backend changes. Just the sizes will show - instead of a size. Then once the backend updates they'll show 0KB until new versions are uploaded.

Testing this fully before the backend updates is a bit hard, requiring pulling my forks of the frontend and backend, as this relies on my changes that are pending for the backend. In order to test with my backend changes, my frontend changes are needed. (Unless you manually make requests to the local backend, which one is required anyways)

Also I have no idea about really anything tailwind, electron react, or typescript. I've learned some typescript from the frontend and backend. and tailwind css from googling and looking at the docs. If there's any improvement to be made for better implementing anything, like the oversized mod tooltip, do let me know.

Implementation

Added Size between Latest and Description.
Added the size to mod-item, with checks for being over 50MB and 100MB.
Added tooltip when over 50MB warning of large mod, and over 100MB for very large mod
Added fileSize to BbmModVersion as the api will be returning fileSize with modVersions

Screenshots

before	after

How to Test

If the backend updates before testing this PR, the easy test should fully work, making no need for the local test

Local test (Harder)

Alright, first clone my fork of the backend, and select the some-testing-for-bsmanager branch (lol)
Run and close it to generate config.json
Set devmode and authBypass to true
Change the MODS_REPO_URL in beatmods-api to http://localhost:5001, the default for the backend
Also clone my fork of the frontend, and change the api in the .env to the same
Run the frontend and backend
Use something like Insomnia to make a request to the backend to add a game version (Ideally one you have in bsmanager already)
Upload and approve a test mod with a zip over 50MB or 100MB (After making sure the frontend is connecting to the local backend, not the public one!)
run this PR and go to the mods tab of the game version you added to the local backend
There should only be the test mod you uploaded, and it should show the size in yellow if between 50MB and 100MB, or red if over 100MB

Make a comment if something isn't working right trying to test

Easy test (Doesn't properly size and warning working)

Run this PR
Go to a mods tab of an installed version
Should be Size at the top, and - for each mod

This implements the changes made in PR Zagrios#48 of the beatmods backend to add file sizes to mods. The change also allows for warning if a mod is oversized. Most mods shouldn't have a warning, but should a mod go over a limit, it'll warn the user. THIS WILL REQUIRE FURTHER TESTING ONCE THE BACKEND UPDATES

It's just easier to test when rebuilding is needed to have one command for build and start. It literally just calls build and start one after the other lol

This reverts commit 28c71be.

Forgor to set these back before committing... oops

Should probably add the other translations for size as well. I've checked that it's the right translation when talking about file size specifically

silentrald · 2025-02-07T07:55:47Z

src/renderer/index.css

@@ -150,3 +150,38 @@
        background-position: 0 0%;
    }
 }
+
+/* Tooltip container */
+.tooltip {


Remove this selector, unless you missed adding stuff here, and probably lint the file, so its align with the old code.

I did run a lint if that's what you mean. It was just 3 warnings, that had nothing to do with anything I did, and an error, which was because of a change I did for testing. (Which I since realized I didn't need for testing)

silentrald · 2025-02-07T07:57:29Z

src/renderer/index.css

+  }
+
+  /* Tooltip text */
+  .tooltip .tooltiptext {


I think this is convertible to tailwind tbh, but check if thats much better than adding manual css.

Do you have any ideas or guidance for converting? I have no idea what I'm doing when it comes to tailwind, other than the little bit I've googled and seen in the docs. I really only know manual css, from doing ASP.NET sites.

Like it took me a total of about 8 hours to figure out adding the Size column to the table without breaking everything. Just to figure out all I needed was to add another content-min to a line

silentrald · 2025-02-07T08:11:49Z

src/renderer/components/version-viewer/slides/mods/mod-item.component.tsx

+            <span className={`min-w-0 text-center bg-inherit py-2 px-1 text-sm border-t-2 border-b-2 group-hover:brightness-90 ${(mod.version.fileSize/1024/1024 > 100 ? "text-red-400 tooltip" : (mod.version.fileSize/1024/1024 > 50 ? "text-yellow-400 tooltip" : "") || "")}`} style={wantInfoStyle}>
+                {(mod.version.fileSize/1024 > 1024 ? `${Math.round(mod.version.fileSize/1024/1024)}MB` : (`${Math.round(mod.version.fileSize/1024)}KB` === `NaNKB` ? `-` : `${Math.round(mod.version.fileSize/1024)}KB`) || "-")}
+                <span className={(mod.version.fileSize/1024/1024 > 100 ? `tooltiptext w-[160px] bg-black text-red-400` : (mod.version.fileSize/1024/1024 > 50 ? `tooltiptext w-[140px] bg-black text-yellow-400` : "") || "")}>
+                    {(mod.version.fileSize/1024/1024 > 100 ? `This is a very large mod!` : (mod.version.fileSize/1024/1024 > 50 ? `This is a large mod!` : "") || "")}
+                </span>
+            </span>


Don't add logic on the jsx part of the code, had a hard time reading this. You might refactor some checks as something like isMediumModSize and isLargeModSize so its much readable and can easily edit what medium and large sizes are.

For the file size part, can you create a function that returns the appropriate format of the file size.
ie. I give it 1500 bytes gives back ~1.5kb
Or 1000000 bytes gives back ~1.0mb sort of thing, if I read the code correctly. Although I might double check your math since === "NaNkb" is a weird equality

Ya ig probably could do with some refactoring.
The NaNKB is because that's what it ends up being when there's no fileSize, like before the backend updates to report one.
I'm about to head to bed, so I'll work on refactoring and cleanup once I've had some sleep.

Try to handle the NaN or any weird stuff first, before computing for the formatted file size ig

That would be better... I've mostly been writing at or after midnight, I need to fix my sleep schedule.
It also is mostly just for if this is pulled and an update goes public before the backend updates. But still good to have the check just in case

Refactored the code for checking and showing mod size. Also redid manual css in tailwind css

TheUnifox · 2025-02-08T03:26:00Z

Done the refactor and moving of css to tailwind

silentrald · 2025-02-08T03:33:59Z

src/renderer/components/version-viewer/slides/mods/mod-item.component.tsx

+    const isMediumMod = verifyFileSize && fileSize/1024/1024 > 50;
+
+    const isLargeMod = verifyFileSize && fileSize/1024/1024 > 100;


Do the reverse of this. Something like fileSize > 1024 * 1024 * 100; // 100mb

silentrald · 2025-02-08T03:45:10Z

src/renderer/components/version-viewer/slides/mods/mod-item.component.tsx

+        if (fileSize/1024 > 1024)
+            return `${Math.round(mod.version.fileSize/1024/1024)}MB`;
+        return `${Math.round(mod.version.fileSize/1024)}KB`;


Do the same change here as well. You can also show 2 decimal places to atleast show some precise approximation.

if (fileSize < 1024 * 1024) return size in KB; return size in MB;

silentrald · 2025-02-08T03:45:13Z

src/renderer/components/version-viewer/slides/mods/mod-item.component.tsx

@@ -51,27 +51,49 @@ export function ModItem({ className, mod, installedVersion, isDependency, isSele
        onChange(!isChecked);
    };

+    const {fileSize} = mod.version;


Don't destruct this anymore, just access the mod.version.fileSize as is in the codebase.

I did have it just access, but it complained saying to use destructing instead. Just ignore the warning here then?

Actually, this is kinda possible with a ReactHook if you know how to do it.

For example

function useFileSize({ fileSize }: Props<{ fileSize?: number }>) { // add the logic here // return { isMediumMod, isLargeMod, getFormattedSize, renderFileSize, // <-- if you can move the rendering here }; } // main component const { isMediumMod, isLargeMod, getFormmattedSize, renderFileSize } = useFileSize({ fileSize: mod.version.fileSize });

But if the logic can be contained in a component itself, might just do a local component.

function FileSizeText({ fileSize }: Props<{ fileSize?: number }>) { // add the logic here // return /* your jsx code here */; } // in the main jsx rendering <FileSizeText fileSize={mod.version.fileSize} />

I don't know much, but which hook should it be? useState?

I've edited the comment above, check the implementation that fits, I'm leaning towards the 2nd btw.

silentrald · 2025-02-08T03:49:48Z

src/shared/models/mods/mod.interface.ts

@@ -42,6 +42,7 @@ export interface BbmModVersion {
    downloadCount: number;
    lastApprovedById?: number;
    lastUpdatedById?: number;
+    fileSize: number;


Add a question mark here as well, since it can be undefined.

TheUnifox · 2025-02-08T05:42:14Z

Changed things around, moved the file size to separate file, and redid tooltip to use Tippy

silentrald · 2025-02-08T05:59:12Z

@TheUnifox can you move the code from "file-size-text.tsx" to "mod-item.component.tsx" since the logic there is very much within only that component and will not be used by any other that I foresee. Will approve the PR once that's done.

Also try to resolve the sonarqubecloud complains, if you can as well.

Since FileSizeText will only ever be used on ModItems

sonarqubecloud · 2025-02-08T06:15:32Z

Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

silentrald

Approved (ﾉ^ヮ^)ﾉ* gj gj.

You'll still wait approval from Zagrios before this get merged but thanks you for the contribution.

TheUnifox · 2025-02-08T06:20:09Z

Yippee :3
It can either wait to be pulled for the backend PR to be pulled. Or it can be pulled whenever, and it'll already be ready to go when the backend updates

GaetanGrd · 2025-02-08T16:53:37Z

Hey, I like the display of file sizes, which fits in quite well with what's already there. However, one thing personally bothers me: it's the use of the color red to show the warning. I would be more inclined to put the text in yellow and/or add an icon to catch the eye. Red is often reserved for “errors” or to indicate a problem.

It might also be preferable to keep the tippy text in white to remain consistent with other warnings, like this:

Note that this is just my personal opinion; you have every right not to agree with me. x)

TheUnifox · 2025-02-08T21:02:23Z

However, one thing personally bothers me: it's the use of the color red to show the warning. I would be more inclined to put the text in yellow and/or add an icon to catch the eye. Red is often reserved for “errors” or to indicate a problem.

Red is only when it's over 100MB, when it's between 50MB and 100MB it is yellow. When under 50MB it's just plain white. I did originally want to do an icon, but ran into issues. Tho now that those issues are figured out it could be added

TheUnifox added 5 commits February 6, 2025 01:46

Add build_start script

28c71be

It's just easier to test when rebuilding is needed to have one command for build and start. It literally just calls build and start one after the other lol

Revert "Add build_start script"

258931d

This reverts commit 28c71be.

Revert testing changes

1e7dde6

Forgor to set these back before committing... oops

Add translations for Size

07f2455

Should probably add the other translations for size as well. I've checked that it's the right translation when talking about file size specifically

silentrald suggested changes Feb 7, 2025

View reviewed changes

TheUnifox and others added 2 commits February 7, 2025 20:24

Refactor + move css to tailwind

af6e205

Refactored the code for checking and showing mod size. Also redid manual css in tailwind css

Merge branch 'master' into feature/fileSizes+OversizeWarning

28407c5

silentrald suggested changes Feb 8, 2025

View reviewed changes

silentrald reviewed Feb 8, 2025

View reviewed changes

TheUnifox added 2 commits February 7, 2025 21:25

A bit more refactoring

37538f6

Use Tippy for tooltip

187174a

Move file-size-text.tsx into mod-item

3a2c926

Since FileSizeText will only ever be used on ModItems

silentrald approved these changes Feb 8, 2025

View reviewed changes

GaetanGrd assigned TheUnifox Feb 8, 2025

GaetanGrd requested review from silentrald and Zagrios February 8, 2025 16:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feat] Add file sizes+oversize warning #791

[feat] Add file sizes+oversize warning #791

TheUnifox commented Feb 7, 2025

silentrald Feb 7, 2025

TheUnifox Feb 7, 2025

silentrald Feb 7, 2025

TheUnifox Feb 7, 2025

TheUnifox Feb 7, 2025

silentrald Feb 7, 2025

TheUnifox Feb 7, 2025

silentrald Feb 7, 2025

TheUnifox Feb 7, 2025

TheUnifox commented Feb 8, 2025

silentrald Feb 8, 2025

silentrald Feb 8, 2025

silentrald Feb 8, 2025

TheUnifox Feb 8, 2025

silentrald Feb 8, 2025 •

edited

Loading

TheUnifox Feb 8, 2025

silentrald Feb 8, 2025

silentrald Feb 8, 2025

TheUnifox commented Feb 8, 2025

silentrald commented Feb 8, 2025 •

edited

Loading

sonarqubecloud bot commented Feb 8, 2025

silentrald left a comment

TheUnifox commented Feb 8, 2025

GaetanGrd commented Feb 8, 2025

TheUnifox commented Feb 8, 2025

		const isMediumMod = verifyFileSize && fileSize/1024/1024 > 50;

		const isLargeMod = verifyFileSize && fileSize/1024/1024 > 100;

[feat] Add file sizes+oversize warning #791

Are you sure you want to change the base?

[feat] Add file sizes+oversize warning #791

Conversation

TheUnifox commented Feb 7, 2025

Scope

Implementation

Screenshots

How to Test

Local test (Harder)

Easy test (Doesn't properly size and warning working)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheUnifox commented Feb 8, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

silentrald Feb 8, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheUnifox commented Feb 8, 2025

silentrald commented Feb 8, 2025 • edited Loading

sonarqubecloud bot commented Feb 8, 2025

Quality Gate passed

silentrald left a comment

Choose a reason for hiding this comment

TheUnifox commented Feb 8, 2025

GaetanGrd commented Feb 8, 2025

TheUnifox commented Feb 8, 2025

silentrald Feb 8, 2025 •

edited

Loading

silentrald commented Feb 8, 2025 •

edited

Loading