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

Fixes DDEV search component. #147

Merged

Conversation

bmartinez287
Copy link
Collaborator

@bmartinez287 bmartinez287 commented Nov 21, 2023

The Issue

The issue is described here

How This PR Solves The Issue

It adds the domain to the javascript search utility.

Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8590de9
Status: ✅  Deploy successful!
Preview URL: https://a0a48f19.ddev-com-front-end.pages.dev
Branch Preview URL: https://20231120-bmartinez287-ddev-p.ddev-com-front-end.pages.dev

View logs

@bmartinez287
Copy link
Collaborator Author

After looking at a few alternatives such as creating a constant, or saving the domain on the search.json file this seemed to be the quickest and potentially the most elegant fix. @rfay

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

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

Well, it sure works and it sure seems elegant! Thanks.

@bmartinez287
Copy link
Collaborator Author

I will merge it in since it's already broken, but if you have a suggestion for a fix that's a bit better let us know. @mattstein. I think it originally worked because it made some assumptions that the recent major upgrade took away.

@rfay
Copy link
Member

rfay commented Nov 21, 2023

OK, sounds like a plan. We can go back to it if Matt has a better idea.

@bmartinez287 bmartinez287 merged commit bf27f6f into main Nov 21, 2023
2 checks passed
@bmartinez287 bmartinez287 deleted the 20231120_bmartinez287_ddev_pr_fix_search_component branch November 21, 2023 13:53
@mattstein
Copy link
Collaborator

Oof, thanks for fixing this @bmartinez287 and sorry I broke it!

@bmartinez287
Copy link
Collaborator Author

No worries, thanks for keeping the project up to date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants