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

Rewrite prose for JavaScript disabled message #3684

Merged
merged 2 commits into from
Feb 3, 2021

Conversation

cloudrac3r
Copy link
Contributor

Description

This should improve the clarity of the message and make it easy to
understand what the problem is at a glance.

  • Clearly show that the page is PeerTube, without having to read the whole prose
  • Clearly show that the problem is JavaScript, without having to read the whole prose
  • Reorganise suggestions into a bullet list

I figured this was trivial enough that I didn't need to open an issue for discussion first. Just let me know if you have comments :)

Has this been tested?

Yes, manually, by loading the HTML file in a browser with scripts disabled, and checking that the page looked like what I wanted.

This should improve the clarity of the message and make it easy to
understand what the problem is at a glance.

- Clearly show that the page is PeerTube, without having to read the
  whole prose
- Clearly show that the problem is JavaScript, without having to read
  the whole prose
- Reorganise suggestions into a bullet list
Copy link
Collaborator

@rigelk rigelk left a comment

Choose a reason for hiding this comment

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

I changed a few things along the way to simplify further maintenance.

@rigelk rigelk merged commit 2ca7235 into Chocobozzz:develop Feb 3, 2021
@rigelk
Copy link
Collaborator

rigelk commented Feb 3, 2021

@cloudrac3r thanks! This is indeed simpler to read and act upon.

@cloudrac3r
Copy link
Contributor Author

Did you test your changes? I found that noscript would be replaced with a span. So it couldn't be styled directly, and max-width wouldn't apply anyway since the element was inline. That's why I added the nested div.

@rigelk
Copy link
Collaborator

rigelk commented Feb 3, 2021

yes, I tested them the same way you did.

@cloudrac3r
Copy link
Contributor Author

I can't get it to apply the styles, but I trust that you know more than me, so you're probably doing it right and I'm not. :) Thanks!

@rigelk
Copy link
Collaborator

rigelk commented Feb 3, 2021

what browser are you testing this with? Are you running the instance in a specific mode (dev? prod?)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants