-
Notifications
You must be signed in to change notification settings - Fork 156
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: visual redesign and removal of obsolete content #445
Conversation
✅ Deploy Preview for webmonetization-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
fd3c3a9
to
96a5c5d
Compare
d01e8f1
to
45898b3
Compare
@@ -0,0 +1,17 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modified from withastro/starlight#1675 (comment)
9c30cdf
to
6e0358b
Compare
ca75c70
to
5c7ef33
Compare
76484a0
to
07dbdc6
Compare
df0f45b
to
6dc82f1
Compare
6dc82f1
to
36f096a
Compare
@@ -1,5 +1,5 @@ | |||
--- | |||
const { tools } = Astro.props; | |||
const { plugins } = Astro.props; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed this to plugins because Arpi's tools are coming soon
@@ -48,7 +48,7 @@ export default function LinkTagGenerator() { | |||
}} | |||
/> | |||
<button | |||
className={styles.btn} | |||
className='btn' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved btn
styles into shared styles instead
@@ -60,9 +60,9 @@ export default function LinkTagGenerator() { | |||
</button> | |||
</div> | |||
{invalidUrl ? ( | |||
<span className={styles.error}> | |||
<p className='error'> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for this
@@ -18,24 +18,3 @@ | |||
border: 1px solid var(--sl-color-gray-2); | |||
padding: var(--space-2xs); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to webm.css since it's shared
@@ -10,15 +10,15 @@ const compatData = json.api | |||
<td></td> | |||
<th class="desktop" colspan='6' title='desktop'> | |||
<img | |||
src={`/img/icon-desktop.svg`} | |||
src={`/img/components/browsercompat/icon-desktop.svg`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole file is path updates because I reorganised the img folder
@@ -0,0 +1,19 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used in the "new" home page that you can see at /index-beta
@@ -0,0 +1,23 @@ | |||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also used for "new" home page at /index-beta
@@ -21,16 +21,13 @@ export function ImportView() { | |||
setError('') | |||
}} | |||
/> | |||
<p className={styles.error}>{error}</p> | |||
<p className='error'>{error}</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shared styles can be found in webm.css
@@ -55,22 +55,6 @@ | |||
margin-block-end: var(--space-m); | |||
} | |||
|
|||
.btn { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Styles moved to webm.css
@@ -6,4 +6,4 @@ tableOfContents: false | |||
import Libraries from '/src/components/docs/Libraries.astro' | |||
import data from '/src/data/home.json' | |||
|
|||
<Libraries tools={data.tools} /> | |||
<Libraries plugins={data.plugins} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JSON key renamed from tools
to plugins
@@ -1,107 +1,106 @@ | |||
{ | |||
"browsers": [], | |||
"providers": [], | |||
"search": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reworked this JSON data file to remove the obsolete bits
@@ -10,6 +10,9 @@ changeLanguage("en"); | |||
|
|||
const title = t("site.title"); | |||
const description = t("site.description"); | |||
const pluginsList = data.plugins.filter(function(plugin){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No point displaying all the old ones. I've added text to send people to the full list if they want to help with the updating
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Can we remove the footer from the playground?
- The WM logo that was present in the playground is now missing. It is only displayed as text. If this was intentional - I am okay with this approach.
The welcome screen looks good! A few comments for the playground.
</div> | ||
<Base title={title} description={description}> | ||
<section class="section"> | ||
<div class="contentWrapperMd"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to utilize all the available width. Can we remove the contentWrapperMd
or update so it doesn't have a max width and add an inline padding of 2rem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -54,113 +24,42 @@ changeLanguage('en') | |||
required | |||
/> | |||
</div> | |||
<button class='full-width' type='submit' disabled> | |||
<button class='btn full-width' type='submit' disabled> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The initial button had a padding of 0.75rem
and the font size was higher as well. The previous one was more prominent and I think we were using the default font size for the button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i've bumped the font size back up. the previous button styles used the smaller text, but your point is good, buttons should be prominent.
margin-block: 0.5rem; | ||
padding-block-start: 0.5rem; | ||
padding-inline-start: 0.5rem; | ||
margin-block: var(--space-2xs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text in the ul
is significantly higher on a desktop screen size. I feel like we can make it a bit smaller. This is probably coming from the applied styles to main
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i've tried to make it smaller, i think it kinda matches the current version now
@huijing going through the new site I found this in
Shouldn't the link be: Perhaps we can just as well fix that here too. |
You mentioned fixing #460 but I still see |
Also just wondering what happened to
|
src/data/home.json
Outdated
{ | ||
"name": "Fynbos", | ||
"image": "logo-wallet-fynbos.svg", | ||
"link": "https://fynbos.app/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"link": "https://fynbos.app/" | |
"link": "https://wallet.fynbos.app/wallet" |
You're linking to their investment page not the wallet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing.
src/pages/prob-revshare.astro
Outdated
@@ -13,18 +13,23 @@ const description = t("site.description"); | |||
<div class="contentWrapperMd"> | |||
<h1 class="heading2">Probabilistic Revshare Generator</h1> | |||
<p>Probabilistic revenue sharing (revshare) is one way to share a portion of a web monetized page’s earnings between multiple payment pointers.</p> | |||
<p>Use this tool to define a list of payment pointers and their weights. Then, add the generated monetization <code>link</code> element to your site. The <code>link</code> will contain a unique URL hosted on <code>https://webmonetization.org/api/revshare/pay/</code>. If you'd prefer to not use a hosted URL, you can set up revshare by <a href="/docs/guides/set-up-probabilistic-revenue-sharing" target="_blank">adding a script to your site</a>.</p> | |||
<p>Use this tool to define a list of payment pointers and their weights. Then, add the generated monetization <code>link</code> element to your site. The <code>link</code> will contain a unique URL hosted on <code>https://webmonetization.org/api/revshare/pay/</code>. If you'd prefer to not use a hosted URL, you can set up revshare by <a href="/docs/resources/set-up-probabilistic-revenue-sharing" target="_blank">adding a script to your site</a>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surely we need to still move these files if we want the new links to work? They're not working for me, but the files are still in the guides folder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixing
Re "Web Monetization Enabled Search Engines": Mojeek's site appears to only have the meta tag, not the link tag. So I'm not sure we have any WM-enabled search engines at the moment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Playground and welcome screen looks good!
I realise this is also kind of out of scope of this PR but I'm also wondering if we have the correct link here? Shouldn't it rather be https://wicg.io/? |
src/components/docs/Footer.astro
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't find where this is used. Can you help me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
u're right. it's not used.
</div> | ||
</section> | ||
|
||
<section class="section"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can have the LinkCards stack vertically on smaller screens?
And can we add a aria-hidden="true"
to the svgs in the cards?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following @melissahenderson comment, perhaps we should remove the "Web Monetization Enabled Search Engines". Sorry for duplicating work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here to remove the "Web Monetization Enabled Search Engines" section and also the Mojeek logo then.
9357701
to
8c90c6d
Compare
Closes #444
Will also resolve #460
This will end up being a mega-PR where the home page is completely re-worked.Is currently a fork of @melissahenderson's working branch and will be constantly rebased as that branch is updated.Plans change. ¯\_(ツ)_/¯
Potential new chronology of events might be:1. Launch of WM alpha extension2. Add docs pages related to alpha, i.e. how to get set up with testnet, screenshots of how to configure extension (potentially in conjunction with home page update)3. Update home page (this PR)4. WM beta extension is available5. mh branch gets publishedFor now, just send good thoughts.Okay, we've changed plans yet again. This PR has been open for way too long, some people's infants have probably started walking by now.
The long and short of it is, since @raducristianpopa has added 2 pages with the new design already, it highlighted the fact that the new design is ready and has been ready for months now. There is no reason to not roll it out. Therefore, we are separating the content rewrite from the redesign.