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

Add select explainer redirect #1101

Merged
merged 4 commits into from
Nov 24, 2024
Merged

Conversation

josepharhar
Copy link
Collaborator

css tricks has an article which is pointing to a 404 on our site here: https://css-tricks.com/the-selectmenu-element/#:~:text=Based%20on%20the%C2%A0Open%20UI%E2%80%99s%C2%A0%3Cselect%3E%C2%A0proposal

This patch adds a page with a link to the updated explainer to make sure we don't continue to 404.

css tricks has an article which is pointing to a 404 on our site here:
https://css-tricks.com/the-selectmenu-element/#:~:text=Based%20on%20the%C2%A0Open%20UI%E2%80%99s%C2%A0%3Cselect%3E%C2%A0proposal

This patch adds a page with a link to the updated explainer to make sure
we don't continue to 404.
@mfreed7
Copy link
Collaborator

mfreed7 commented Sep 27, 2024

This is "ok" like this, but perhaps better is a server redirect?

See this URL for an example:

https://open-ui.org/components/popup.research.explainer/

It automatically redirects (without an interstitial and extra click) over to

https://open-ui.org/components/popover.research.explainer/

@josepharhar
Copy link
Collaborator Author

This is "ok" like this, but perhaps better is a server redirect?

See this URL for an example:

https://open-ui.org/components/popup.research.explainer/

It automatically redirects (without an interstitial and extra click) over to

https://open-ui.org/components/popover.research.explainer/

Yes that is much better, I found a configuration file with redirects in it, and I replaced the selectlist and selectmenu pages with redirects.

When I ran it locally it didn't look like it was working though. Maybe I'm missing something?

@mfreed7
Copy link
Collaborator

mfreed7 commented Oct 1, 2024

This is "ok" like this, but perhaps better is a server redirect?
See this URL for an example:
https://open-ui.org/components/popup.research.explainer/
It automatically redirects (without an interstitial and extra click) over to
https://open-ui.org/components/popover.research.explainer/

Yes that is much better, I found a configuration file with redirects in it, and I replaced the selectlist and selectmenu pages with redirects.

When I ran it locally it didn't look like it was working though. Maybe I'm missing something?

Interesting. Looking at the source files, it looks like someone changed how this was working since the last time I touched it also. I wonder who the expert would be?

Side-thing, I also notice as I look back at the popup/popover files, there are still old versions of the "popup" explainer lingering in the source tree. That's likely bad too?

@josepharhar
Copy link
Collaborator Author

I wonder who the expert would be?

Probably @andrico1234

Side-thing, I also notice as I look back at the popup/popover files, there are still old versions of the "popup" explainer lingering in the source tree. That's likely bad too?

Are you talking about this? I figured it was needed to make the redirect work but we could try deleting it and see what happens? https://github.com/openui/open-ui/blob/main/site/src/pages/components/popup.research.explainer.js

@mfreed7
Copy link
Collaborator

mfreed7 commented Oct 15, 2024

I wonder who the expert would be?

Probably @andrico1234

Side-thing, I also notice as I look back at the popup/popover files, there are still old versions of the "popup" explainer lingering in the source tree. That's likely bad too?

Are you talking about this? I figured it was needed to make the redirect work but we could try deleting it and see what happens? https://github.com/openui/open-ui/blob/main/site/src/pages/components/popup.research.explainer.js

No, not that one. I do think that's needed for the redirect? I meant the other popup.*.mdx files. But after reviewing them, I remember that they're all "historical" versions that are kept for posterity. No issue, I think.

@josepharhar
Copy link
Collaborator Author

Ok cool. Anything else I can do to get this merged? I still don't have merging permissions.

@gregwhitworth gregwhitworth merged commit 0259f7b into openui:main Nov 24, 2024
5 checks passed
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.

5 participants