-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Website: Rewrite index with filters #6483
Conversation
File
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't gone over this yet, but I would like to before it's merged.
Hi @SamWilsn thank you for checking in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be split into two PRs. One to add the RSS feeds, the other to add filters.
I agree there are two concerns in this PR. Maybe they are related maybe they are not. Don't anybody have feedback yet on the PR as-is? And either part of it? |
As previously mentionned, the RSS feeds should be a separate PR. I like the idea of filters in concept, but I would prefer it to be a separate page, instead of overriding the readme. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the CI errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+0. I like the filters, but I'd rather it not replace the main index.
@Pandapip1 Is your branch up-to-date? Can I take your index (the rendered static HTML version) and use that in this PR as the homepage? Then I'll move what is currently the homepage here to all.html. Maybe that's a way to move this PR forward -- and then on a later day separately discuss the merits of the EIPs homepage just being all content. |
I'm +1 to that. |
May I have the deployed link again please |
Just use the one here: |
@Pandapip1 got it You asked me to separate out this part #6579 is that ready to merge now? |
No. It needs one more review. |
Will this hurt what little SEO we have? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep the all.html page, since removing it would make the EIPs website significantly harder to index.
9fc7506
to
4693f75
Compare
The commit 4693f75 (as a parent of 9c72bac) contains errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the CI errors.
add page fragments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're going to replace the all.html page with the filter page, then I request that you re-add the category-specific pages.
Also, there is some linking to those pages that would break, which would also be bad for SEO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the RSS feeds are still in this PR.
There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review. |
This pull request was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment. |
New features:
https://youtu.be/gzZqgACuULQ