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

Malay: New language, homepage #4747

Merged
merged 6 commits into from Dec 22, 2021
Merged

Malay: New language, homepage #4747

merged 6 commits into from Dec 22, 2021

Conversation

wackerow
Copy link
Member

@wackerow wackerow commented Dec 11, 2021

Description

  • Adds Malay (ms) to list of languages

  • Import homepage strings from Crowdin

  • Utilizes updated legacy homepage logic from Arabic: Homepage updated #4723

  • Check banner logic

@github-actions github-actions bot added content 🖋️ This involves copy additions or edits review needed 👀 translation 🌍 This is related to our Translation Program labels Dec 11, 2021
@gatsby-cloud
Copy link

gatsby-cloud bot commented Dec 11, 2021

Gatsby Cloud Build Report

ethereum-org-website-dev2

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 7m

Performance

Lighthouse report

Metric Score
Performance 🔶 21
Accessibility 💚 97
Best Practices 💚 93
SEO 🔶 79

🔗 View full report

@gatsby-cloud
Copy link

gatsby-cloud bot commented Dec 11, 2021

Gatsby Cloud Build Report

ethereum-org-website-dev

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 8m

Performance

Lighthouse report

Metric Score
Performance 🔶 22
Accessibility 💚 97
Best Practices 💚 93
SEO 🔶 79

🔗 View full report

@minimalsm
Copy link
Contributor

@wackerow where are we at with this 😀?

gatsby-node.js Outdated
@@ -334,6 +334,9 @@ exports.createPages = async ({ graphql, actions, reporter }) => {
const markdownPath = `${__dirname}/src/content/translations/${lang}/${page}/index.md`
const langHasOutdatedMarkdown = fs.existsSync(markdownPath)
if (!langHasOutdatedMarkdown) {
// Check if json strings exists for language, if not mark `isContentEnglish` as true
const jsonPath = `${__dirname}/src/intl/${lang}/page-${page}.json`
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we just use the existing helper function checkIsPageOutdated for this? cc @corwintines

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also not sure if this check is needed at all, given that we perform checkIsPageOutdated within onCreatePage, although I'm not certain that is triggered when we call createPage below this.

Copy link
Member Author

@wackerow wackerow Dec 21, 2021

Choose a reason for hiding this comment

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

I don't believe it does trigger it from that method itself. exports.onCreatePage calls createPage() itself and would appear to cause a recursive loop if that method also triggered the lifecycle hook. Placing this here fixes the "pages-conditional" that were having the issues.

I updated to use checkIsPageOutdated, and merged in dev / cleared conflicts.

Copy link
Member

@corwintines corwintines left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for catching the page-conditional thing @wackerow!

@corwintines corwintines merged commit c973682 into dev Dec 22, 2021
@corwintines corwintines deleted the ms-homepage branch December 22, 2021 18:33
@wackerow wackerow mentioned this pull request Dec 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content 🖋️ This involves copy additions or edits translation 🌍 This is related to our Translation Program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants