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.

Sign up for GitHub

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

Jump to bottom

qs web-components #58

Merged
merged 1 commit into from
Apr 2, 2024
Merged

qs web-components #58

merged 1 commit into from
Apr 2, 2024

Conversation

ia3andy
Copy link
Collaborator

@ia3andy ia3andy commented Nov 21, 2023

No description provided.

yrodiere
yrodiere reviewed Nov 22, 2023
View reviewed changes
Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

Thanks. And thanks for trying to keep some of my code, that's kind of you.

I added a few comments, hopefully some of them are useful :)

src/main/java/io/quarkus/search/app/SearchService.java Outdated Show resolved Hide resolved
src/main/java/io/quarkus/search/app/dto/SearchHit.java Outdated Show resolved Hide resolved
src/main/resources/web/app/qs-form.ts Outdated Show resolved Hide resolved
src/main/resources/web/app/qs-target.ts Outdated Show resolved Hide resolved
src/main/java/io/quarkus/search/app/hibernate/InputProviderHtmlBodyTextBridge.java Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
src/main/resources/web/app/qs-form.ts Outdated Show resolved Hide resolved
yrodiere
yrodiere reviewed Nov 23, 2023
View reviewed changes
Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

Thanks, looking great.

A few comments below :)

src/main/resources/web/app/assets/icons/docsicon-concepts.svg Outdated Show resolved Hide resolved
src/main/java/io/quarkus/search/app/SearchService.java Outdated Show resolved Hide resolved
src/main/java/io/quarkus/search/app/WebComponents.java Outdated Show resolved Hide resolved
src/main/resources/web/app/qs-guide.ts Outdated Show resolved Hide resolved
src/main/resources/web/app/qs-guide.ts Outdated Show resolved Hide resolved
src/main/resources/web/app/qs-guide.ts Outdated Show resolved Hide resolved
@ia3andy
Copy link
Collaborator Author

ia3andy commented Nov 23, 2023

Thanks @yrodiere for the review!!!

@ia3andy
Copy link
Collaborator Author

ia3andy commented Nov 23, 2023

Ok, I've pushed again:

  • switched to html (and reverted the service changes)
  • added a loading indicator
  • added the content section

For the icons copyright, I've used the one from quarkusio so we should mimic what they are doing?

@yrodiere
Copy link
Member

For the icons copyright, I've used the one from quarkusio so we should mimic what they are doing?

Yep, makes sense

@ia3andy ia3andy marked this pull request as ready for review November 27, 2023 09:49
@yrodiere
Copy link
Member

Hey @ia3andy I see you marked this as ready for review, but just to make sure we agree: you intend to make some more changes, right? Regarding the copyright mentions in particular, and maybe the abort thing (see earlier reviews).

I do understand javascript search will be handled elsewhere.

@ia3andy
Copy link
Collaborator Author

ia3andy commented Nov 30, 2023

Hey @ia3andy I see you marked this as ready for review, but just to make sure we agree: you intend to make some more changes, right? Regarding the copyright mentions in particular, and maybe the abort thing (see earlier reviews).

I do understand javascript search will be handled elsewhere.

Yes sorry I am stuck on code.quarkus, I will get back to this very soon

@yrodiere
Copy link
Member

Hey @ia3andy I see you marked this as ready for review, but just to make sure we agree: you intend to make some more changes, right? Regarding the copyright mentions in particular, and maybe the abort thing (see earlier reviews).
I do understand javascript search will be handled elsewhere.

Yes sorry I am stuck on code.quarkus, I will get back to this very soon

No problem, I just wanted to be sure there wasn't a misunderstanding. Take your time 👍

@yrodiere yrodiere added this to the MVP milestone Nov 30, 2023
@yrodiere yrodiere removed this from the MVP milestone Dec 12, 2023
@yrodiere
Copy link
Member

Hey @ia3andy , since you seem busy, and there are still a few things to fix here, and I think there's still a bit of work to get this integrated with quarkus.io, I'll plan on deploying the first version without web-components. We can always add them later when this is ready, of course.

@yrodiere yrodiere marked this pull request as draft December 20, 2023 15:17
@ia3andy ia3andy mentioned this pull request Jan 8, 2024
Closed
2 tasks
@ia3andy ia3andy mentioned this pull request Feb 21, 2024
Merged
marko-bekhta
marko-bekhta reviewed Mar 19, 2024
View reviewed changes
Copy link
Collaborator

@marko-bekhta marko-bekhta left a comment

Choose a reason for hiding this comment

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

Looks nice! 😃

pom.xml Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
src/main/resources/web/app/qs-guide.ts Outdated Show resolved Hide resolved
yrodiere
yrodiere reviewed Mar 27, 2024
View reviewed changes
Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

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

Thanks, looks nice indeed. I added a few comments below.

src/main/resources/application.properties Outdated Show resolved Hide resolved
src/main/resources/web/app/qs-form.ts Outdated Show resolved Hide resolved
src/main/resources/web/index.html Outdated Show resolved Hide resolved
src/test/java/io/quarkus/search/app/WebComponentsTest.java Show resolved Hide resolved
src/test/java/io/quarkus/search/app/WebComponentsTest.java Outdated Show resolved Hide resolved
@yrodiere yrodiere mentioned this pull request Mar 27, 2024
Merged
@ia3andy ia3andy force-pushed the comp branch 4 times, most recently from a98f9e1 to 5a6c28d Compare March 28, 2024 16:45
@ia3andy ia3andy marked this pull request as ready for review March 28, 2024 17:57
@ia3andy
Copy link
Collaborator Author

ia3andy commented Mar 28, 2024

All good I think

@ia3andy
Copy link
Collaborator Author

ia3andy commented Mar 29, 2024

I think we should merge this, and iterate..

@yrodiere yrodiere merged commit f75feb9 into quarkusio:main Apr 2, 2024
1 check passed
@ia3andy
Copy link
Collaborator Author

ia3andy commented Apr 2, 2024

Yeaaahh!! 🥳

@ia3andy
Copy link
Collaborator Author

ia3andy commented Apr 2, 2024

Now let me complete the quarkusio website!

@yrodiere
Copy link
Member

yrodiere commented Apr 2, 2024

Thanks @ia3andy :)

@ia3andy ia3andy mentioned this pull request Apr 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marko-bekhta marko-bekhta marko-bekhta left review comments

@yrodiere yrodiere yrodiere left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ia3andy @yrodiere @marko-bekhta