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

display image files #7844

Merged
merged 1 commit into from
Jul 5, 2024
Merged

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Jun 27, 2024

Signed-off-by: Philippe Martin phmartin@redhat.com

What does this PR do?

Display the files in an image.

Current limitations:

  • works only if a single provider is installed
  • doc needs to be written to explain the sizes displayed in the layers cards and what does the checkbox Show layer only
  • design to be discussed

Screenshot / video of UI

image-files-explorer.mp4

What issues does this PR fix or reference?

Fixes #7801

How to test this PR?

Start Podman Desktop with the fake extension https://github.com/feloy/podman-desktop-extension-image-files-fake, for example:

yarn watch --extension-folder ../podman-desktop-extension-image-files-fake/

Go to the details page of an image, and select the Files tab.

Navigate through the layers and see the files in this layer.

A checkbox Show layer only gives the choice to:

  • (off) display the state of the filesystem at this layer (considering all the layers up to this one)

  • (on) display only the files added/deleted/modified in this layer

  • Tests are covering the bug fix or the new feature

@feloy feloy requested review from benoitf and a team as code owners June 27, 2024 19:38
@feloy feloy requested review from cdrage, axel7083 and deboer-tim and removed request for a team June 27, 2024 19:38
Signed-off-by: Philippe Martin <phmartin@redhat.com>
@benoitf
Copy link
Collaborator

benoitf commented Jul 2, 2024

just wanted to confirm if we proceed with the UI here or if it goes to a self-contained extension providing its own webview ?

@deboer-tim
Copy link
Collaborator

Personally I think we should provide the ability for extensions to extend details pages with extra tabs using webviews, and things like this, image checker, etc could be provided as extensions unless/until there's a reason to integrate, like if it made sense having multiple providers integrated on the same UI screen. Even developing as a built-in extension would ensure clean arch/API/separation vs continually adding UI to Podman Desktop for one extension to use.

That said, holding this up for a core feature would be a huge impediment so I am fine moving forward as-is. 👍🏼

@jeffmaury
Copy link
Contributor

Personally I think we should provide the ability for extensions to extend details pages with extra tabs using webviews, and things like this, image checker, etc could be provided as extensions unless/until there's a reason to integrate, like if it made sense having multiple providers integrated on the same UI screen. Even developing as a built-in extension would ensure clean arch/API/separation vs continually adding UI to Podman Desktop for one extension to use.

That said, holding this up for a core feature would be a huge impediment so I am fine moving forward as-is. 👍🏼

I think it depends on the nature of the extension: for the image checker, you may have several implementations but I think it does not make much sense that each extension provide its own UI. In this case, I think there will be a single extension so the 2 options are opened

@deboer-tim
Copy link
Collaborator

I think it depends on the nature of the extension: for the image checker, you may have several implementations but I think it does not make much sense that each extension provide its own UI. In this case, I think there will be a single extension so the 2 options are opened

Yeah, totally agreed, but even in this case there is no difference until there are 2 providers. IMHO the 'ideal progression' in cases like this would be starting as a standalone extension until it's clear there are multiple providers and one UI makes sense, and even then it could probably be done via shared extension now. In practice (and even 2 months ago) that's not always feasible, but we need to avoid everything needing to be done in renderer. (and again, not for this PR :) )

Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

works for me as first iteration

Copy link
Collaborator

@deboer-tim deboer-tim left a comment

Choose a reason for hiding this comment

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

Ok merging, but please open up a followup issue to replace hardcoded colors. Especially ones like gray-900, gray-700, but should do layer colors too.

@feloy feloy merged commit 6be8d07 into containers:main Jul 5, 2024
7 checks passed
@podman-desktop-bot podman-desktop-bot added this to the 1.12.0 milestone Jul 5, 2024
rostalan pushed a commit to rostalan/podman-desktop that referenced this pull request Jul 16, 2024
feat: display image files
Signed-off-by: Philippe Martin <phmartin@redhat.com>
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.

Display the files present in an image
5 participants