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

feat: add catalog UI to browse the catalog #72

Merged
merged 5 commits into from
Nov 6, 2024
Merged

feat: add catalog UI to browse the catalog #72

merged 5 commits into from
Nov 6, 2024

Conversation

benoitf
Copy link
Contributor

@benoitf benoitf commented Oct 24, 2024

related to podman-desktop/podman-desktop#8972

Screen.Recording.2024-10-21.at.11.33.25.mov

Change-Id: I820ba559ae561308bca38dc867439615d6e8d8af

@benoitf benoitf requested a review from a team as a code owner October 24, 2024 14:41
@benoitf benoitf requested review from cdrage, deboer-tim and SoniaSandler and removed request for a team October 24, 2024 14:41
@benoitf benoitf mentioned this pull request Oct 24, 2024
Closed
@benoitf benoitf requested review from jeffmaury and axel7083 October 24, 2024 14:42
@benoitf
Copy link
Contributor Author

benoitf commented Oct 24, 2024

reopened the PR on top of the two previous chunks

here there is only the changes for displaying the catalog

jeffmaury
jeffmaury approved these changes Oct 24, 2024
View reviewed changes
Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

LGTM.

Is it expected that the button does not have background like in Podman Desktop ?

@benoitf
Copy link
Contributor Author

benoitf commented Oct 24, 2024

@jeffmaury which button ? install button ?

@jeffmaury
Copy link
Contributor

@jeffmaury which button ? install button ?

Yes

@jeffmaury
Copy link
Contributor

@jeffmaury which button ? install button ?

Yes

FYI:

image

axel7083
axel7083 approved these changes Oct 25, 2024
View reviewed changes
Copy link
Contributor

@axel7083 axel7083 left a comment

Choose a reason for hiding this comment

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

LGTM

@benoitf
Copy link
Contributor Author

benoitf commented Oct 25, 2024

@jeffmaury so it was not expected, in fact all the colors were missing in light theme

I pushed a commit to fix it

@jeffmaury
Copy link
Contributor

@jeffmaury so it was not expected, in fact all the colors were missing in light theme

I pushed a commit to fix it

Seems to be an issue related to Firefox as Edge is fine on Win11Pro
On Firefox, I have the issue on both light and dark themes

@benoitf
Copy link
Contributor Author

benoitf commented Oct 25, 2024

well at least in chrome there was the issue (now fixed)

@benoitf
Copy link
Contributor Author

benoitf commented Oct 25, 2024 •
edited
Loading

firefox is not implementing the @scope feature

@jeffmaury
Copy link
Contributor

firefox is not implementing the @scope feature

Is there a workaround or should we open a follow up issue ?

@benoitf
Copy link
Contributor Author

benoitf commented Oct 25, 2024

I have not seen a workaround for now, anyway even if we merge the PR the website is not published, so we could do a follow-up

jeffmaury
jeffmaury approved these changes Oct 25, 2024
View reviewed changes
Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

LGTM

dgolovin
dgolovin requested changes Oct 28, 2024
View reviewed changes
Copy link
Contributor

@dgolovin dgolovin left a comment

Choose a reason for hiding this comment

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

Copyright comments are missing.

src/lib/api/extensions-info.ts Show resolved Hide resolved
src/lib/ui/ExtensionByCategoryCard.spec.ts Show resolved Hide resolved
src/lib/ui/ExtensionIcon.spec.ts Show resolved Hide resolved
src/lib/ui/ExtensionsByCategory.spec.ts Show resolved Hide resolved
src/lib/ui/ExtensionsDetails.spec.ts Show resolved Hide resolved
src/lib/ui/ExtensionsDetailsBottom.spec.ts Show resolved Hide resolved
src/lib/ui/ExtensionsDetailsTop.spec.ts Show resolved Hide resolved
src/lib/ui/ExtensionsList.spec.ts Show resolved Hide resolved
@benoitf
Copy link
Contributor Author

benoitf commented Nov 4, 2024

README and icon are mandatory, this is why there is no case to handle their missing state on the UI

@benoitf
Copy link
Contributor Author

benoitf commented Nov 4, 2024

@jeffmaury issue should be fixed for Firefox now

@benoitf
Copy link
Contributor Author

benoitf commented Nov 4, 2024

@dgolovin missing copyrights added

@benoitf
feat: add catalog UI to browse the catalog
5270500 
related to podman-desktop/podman-desktop#8972

Signed-off-by: Florent Benoit <[email protected]>
Change-Id: I9e121490ccf65275ec1273f716d9d7a8e26d791a
@benoitf
chore: handle light theme properly
a3b12d1 
Change-Id: I50719b33e95c1453f2fccd39ff4be27cf521616f
Signed-off-by: Florent Benoit <[email protected]>
@benoitf
chore: fix test
bdfb893 
Change-Id: I4a4bc10332329bfa6a2aa3829fdb969615e7ff0a
Signed-off-by: Florent Benoit <[email protected]>
@benoitf
chore: fix css for firefox
aa66958 
Change-Id: I1a74c9251a494ab965c306e9e84716e3b269f7df
Signed-off-by: Florent Benoit <[email protected]>
@benoitf
chore: add missing copyrights
d17c607 
Change-Id: I8c40cbaabb32d87383d602e8774598baf7d11466
Signed-off-by: Florent Benoit <[email protected]>
jeffmaury
jeffmaury approved these changes Nov 5, 2024
View reviewed changes
Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

LGTM

Tested on Firefox

dgolovin
dgolovin approved these changes Nov 5, 2024
View reviewed changes
Copy link
Contributor

@dgolovin dgolovin left a comment

Choose a reason for hiding this comment

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

LGTM

@benoitf benoitf merged commit 3cef4a9 into podman-desktop:main Nov 6, 2024
5 checks passed
@benoitf benoitf mentioned this pull request Nov 7, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgolovin dgolovin dgolovin approved these changes

@jeffmaury jeffmaury jeffmaury approved these changes

@axel7083 axel7083 axel7083 approved these changes

@cdrage cdrage Awaiting requested review from cdrage cdrage was automatically assigned from containers/podman-desktop-reviewers

@deboer-tim deboer-tim Awaiting requested review from deboer-tim deboer-tim was automatically assigned from containers/podman-desktop-reviewers

@SoniaSandler SoniaSandler Awaiting requested review from SoniaSandler SoniaSandler was automatically assigned from containers/podman-desktop-reviewers

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

Successfully merging this pull request may close these issues.
4 participants
@benoitf @jeffmaury @dgolovin @axel7083