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

Fix web metadata #79

Merged
merged 1 commit into from
Feb 5, 2023
Merged

Fix web metadata #79

merged 1 commit into from
Feb 5, 2023

Conversation

JSKitty
Copy link
Member

@JSKitty JSKitty commented Feb 3, 2023

Abstract

Since the merge of #68, some of MPW's metadata (icons, banner) were "lost" as the structure of the files was changed without corrections.

My edit simply removes the path "hardcoding" and makes them relative to the root directory.

Testing

Simply check the metadata display of our live MPW.
Against the metadata display of my modified MPW.

You'll notice the missing brand image from the live site; this should also apply to Discord, but beware that Discord caches this stuff, so even an update MPW site may take some time before Discord registers the new metadata.

Note:

I'm unsure if this is the best fix; since MPW is now essentially "flat" in it's file structure, this probably doesn't index well by search engines.

Should our images be in a public-facing /assets/ directory? Icons? etc?

Open to suggestions, but if none are made, this small fix will suffice.

@JSKitty JSKitty added Bug This is either a bugfix (PR) or a bug (issue). Enhancement New feature or request Minor (~45 PIV) labels Feb 3, 2023
@JSKitty JSKitty requested review from Liquid369 and Duddino February 3, 2023 23:51
@JSKitty JSKitty self-assigned this Feb 3, 2023
Copy link

@Liquid369 Liquid369 left a comment

Choose a reason for hiding this comment

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

tACK 397e09b

I have compared the two.
This works fine but I do not have an opinion in regard to the /assets/ being kept.
In general, seems like it would make sense to keep /assets/ but unsure if it really makes a difference
While testing the MN List work I am doing saw these in the console as errors so looks good overall

Copy link
Member

@Luke-Larsen Luke-Larsen left a comment

Choose a reason for hiding this comment

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

tACK 397e09b

I have compared the two just like liquid, I think the changes and moving the favicon out of assets is good as some older browsers and systems default to it being in root. Everything else looks fine.

@JSKitty JSKitty merged commit 5305968 into PIVX-Labs:master Feb 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This is either a bugfix (PR) or a bug (issue). Enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

3 participants