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

Tweak <noscript> text and placement #2101

Merged
merged 1 commit into from
Jan 9, 2020
Merged

Tweak <noscript> text and placement #2101

merged 1 commit into from
Jan 9, 2020

Conversation

kzys
Copy link
Contributor

@kzys kzys commented Jan 7, 2020

After having FastBoot, JavaScript would not be "required" but still
necessary for full functionality of crates.io.

@rust-highfive
Copy link

r? @carols10cents

(rust_highfive has picked a reviewer for you, use r? to override)

@locks
Copy link
Contributor

locks commented Jan 7, 2020

We might want to still have some kind of message, as some functionality will not be available if you don't have JS enabled. What do you think?

@jtgeibel
Copy link
Member

jtgeibel commented Jan 8, 2020

How about changing the message to something like:

Some functionality may not be available with JavaScript disabled.

Most users won't see the message at all, and with functionality like the download charts we may not ever reach complete feature parity.

We could also move the message to a better place with better styling, as it is currently located in a strange place below the footer.

@kzys
Copy link
Contributor Author

kzys commented Jan 8, 2020

If so, we could probably put the message in app/index.html instead of adding this new file. What do you think?

@jtgeibel
Copy link
Member

jtgeibel commented Jan 8, 2020

Yeah, I agree. I'd like to avoid adding another file that we'll need to keep synchronized with app/index.html.

After having FastBoot, JavaScript would not be "required" but still
necessary for full functionality of crates.io
@jtgeibel jtgeibel changed the title Remove <noscript> from FastBoot-rendered pages Tweak <noscript> text and placement Jan 9, 2020
@jtgeibel
Copy link
Member

jtgeibel commented Jan 9, 2020

LGTM! I've updated the PR subject and text to reflect the changes.

@bors r+

@bors
Copy link
Contributor

bors commented Jan 9, 2020

📌 Commit 2ef458e has been approved by jtgeibel

@bors
Copy link
Contributor

bors commented Jan 9, 2020

⌛ Testing commit 2ef458e with merge 33dfe99...

bors added a commit that referenced this pull request Jan 9, 2020
Tweak <noscript> text and placement

After having FastBoot, JavaScript would not be "required" but still
necessary for full functionality of crates.io.
@bors
Copy link
Contributor

bors commented Jan 9, 2020

☀️ Test successful - checks-travis
Approved by: jtgeibel
Pushing 33dfe99 to master...

@bors bors merged commit 2ef458e into rust-lang:master Jan 9, 2020
@kzys kzys mentioned this pull request Jan 9, 2020
19 tasks
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.

6 participants