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

Update index.html #1502

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update index.html #1502

wants to merge 1 commit into from

Conversation

Ayush277
Copy link

This "selfoss" HTML template is enhanced for performance and accessibility. Key changes include adding lang="en" for better accessibility and a tag to inform users when JavaScript is disabled. The viewport meta tag is streamlined for improved mobile responsiveness, and the stylesheet link is optimized for quicker rendering. JavaScript loads asynchronously, enhancing page performance, with improved error handling directing users to check logs if loading fails. These updates make "selfoss" more efficient and user-friendly across devices.

This "selfoss" HTML template is enhanced for performance and accessibility. Key changes include adding lang="en" for better accessibility and a <noscript> tag to inform users when JavaScript is disabled. The viewport meta tag is streamlined for improved mobile responsiveness, and the stylesheet link is optimized for quicker rendering. JavaScript loads asynchronously, enhancing page performance, with improved error handling directing users to check logs if loading fails. These updates make "selfoss" more efficient and user-friendly across devices.
Copy link

netlify bot commented Oct 26, 2024

Deploy Preview for selfoss canceled.

Name Link
🔨 Latest commit 67c6692
🔍 Latest deploy log https://app.netlify.com/sites/selfoss/deploys/671d2ed6d8d2970008f93de5

@Ayush277
Copy link
Author

Enhanced "selfoss" HTML template for better performance and accessibility. Added lang="en" for accessibility and a tag for users with JavaScript disabled. Streamlined viewport meta tag for mobile responsiveness, optimized stylesheet link for faster loading, and asynchronous JavaScript loading with improved error handling. Overall, these updates make "selfoss" more efficient and user-friendly.

Copy link
Member

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the contribution and sorry for the late response.

Could you please split each individual change into a separate commit so that it is clear to see what actually changed? It would also allow to explain the reasoning for the changes in the relevant commit message.

<meta name="viewport" content="width=device-width, user-scalable=yes, initial-scale=1" />

<!-- Add support for fullscreen Webapp on iPhone 5 -->
<meta name="viewport" content="initial-scale=1, user-scalable=yes" media="(device-height: 568px)" />
Copy link
Member

Choose a reason for hiding this comment

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

Could you clarify why this is no longer needed?

@@ -1,47 +1,41 @@
<!doctype html>
<html>
<html lang="en">
Copy link
Member

Choose a reason for hiding this comment

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

The application can be in different language and is set dynamically here:

if (configuration.language !== null) {
document.documentElement.setAttribute(
'lang',
configuration.language,
);
}

<p id="js-loading-message">selfoss requires JavaScript to run, please enable it on this page.</p>
<noscript><p>selfoss requires JavaScript to run. Please enable it to continue.</p></noscript>

<p id="js-loading-message" aria-live="polite">selfoss is still loading, please wait.</p>
Copy link
Member

Choose a reason for hiding this comment

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

This will cause two messages will be displayed when JavaScript is disabled.


<script type="text/javascript">
<script>
document.getElementById('js-loading-message').textContent = 'selfoss is still loading, please wait.';
Copy link
Member

Choose a reason for hiding this comment

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

In the current state, is not this redundant?

<title>selfoss</title>

<!-- Will be substituted at request time. -->
<!-- Dynamic base path -->
Copy link
Member

Choose a reason for hiding this comment

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

This removes information.

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.

2 participants