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

feat: make us SEO compatible (#745) #790

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

EtayZaslavsky
Copy link
Collaborator

Description

  1. Created sitemap.js script. It creates a sitemap file in public/sitemap.xml dynamically from the url paths in the src/routes/index.tsx file. The urls get a static base_url (https://open-bus-map-search.hasadna.org.il/).
  2. Added "node sitemap.js" to the build process in package.json.
  3. Added the sitemap url to robots.txt.
  4. Added a "/" prefix to one of the urls in src/routes/index.tsx, for the sitemap.xml regex to find it (as all other urls had it).

Copy link
Contributor

github-actions bot commented Jun 9, 2024

@EtayZaslavsky
Copy link
Collaborator Author

EtayZaslavsky commented Jun 9, 2024

Didn't pass lint test. My mistake for not testing locally.

@EtayZaslavsky EtayZaslavsky reopened this Jun 9, 2024
Copy link
Member

Choose a reason for hiding this comment

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

Let's ignore this file on git, it's autogenerated

Copy link
Member

Choose a reason for hiding this comment

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

let's also delete this file

sitemap.js Outdated Show resolved Hide resolved
src/routes/index.tsx Show resolved Hide resolved
@supervxn
Copy link
Collaborator

I have some experience in SEO, how can I help?

Copy link
Member

@NoamGaash NoamGaash left a comment

Choose a reason for hiding this comment

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

Thanks! So let's just ignore the auto-generated sitemap

@EtayZaslavsky
Copy link
Collaborator Author

could it be that these tests failed without connection the code i pushed?

@NoamGaash
Copy link
Member

yes, there's an error in production right now

@EtayZaslavsky
Copy link
Collaborator Author

Is there still a problem?

@NoamGaash
Copy link
Member

let's see. I'll re-run

@EtayZaslavsky
Copy link
Collaborator Author

Hi there :)
is there anything new with the problem?

@NoamGaash
Copy link
Member

Im not sure, let's try

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.

3 participants