Skip to content

frontend: Add Map#2225

Merged
joaquimrocha merged 6 commits intomainfrom
map-graph
Nov 5, 2024
Merged

frontend: Add Map#2225
joaquimrocha merged 6 commits intomainfrom
map-graph

Conversation

@sniok
Copy link
Copy Markdown
Contributor

@sniok sniok commented Aug 1, 2024

This PR introduces a new Map view under /map route
It displays most of the kubernetes resource as a graph

Implements #198


Description of all the changes in this PR

frontend: Add Map page and route

Introduces new resource Map view under /map route
This is the main commit introducing the map under src/components/resourceMap

frontend: Allow Routes to take full width, add isFullWidth route parameter

Map needs as much space as possible.
By default Layout Container will have a max width.
New isFullWidth parameter on the route removes max width


Out of scope for this PR:

  • Tests
  • Exposing Map to plugins
  • Ability to extend map from plugins
  • Full coverage of all resources (some resources are not implemented yet)
  • Full coverage of all possible connections

Testing done

  • Tested with different clusters (k3s, azure aks)
  • Load testing with kwok
  • Listing, filtering, grouping, updating resources

@illume illume changed the title WIP frontend: Add Map WIP frontend: Add Service Map Oct 8, 2024
@illume
Copy link
Copy Markdown
Contributor

illume commented Oct 8, 2024

Related to #198

@sniok sniok force-pushed the map-graph branch 9 times, most recently from 5f98d31 to 2532966 Compare October 15, 2024 10:43
@sniok sniok changed the title WIP frontend: Add Service Map frontend: Add Map Oct 15, 2024
@sniok sniok force-pushed the map-graph branch 4 times, most recently from ebcfee2 to 724548e Compare October 16, 2024 11:53
@sniok sniok marked this pull request as ready for review October 16, 2024 12:11
@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Oct 16, 2024
@sniok sniok requested a review from joaquimrocha October 16, 2024 12:11
@sniok sniok force-pushed the map-graph branch 4 times, most recently from f703405 to cd4de8b Compare October 24, 2024 11:32
Copy link
Copy Markdown
Contributor

@joaquimrocha joaquimrocha left a comment

Choose a reason for hiding this comment

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

Thanks @sniok . I left a few comments. Overall, I think we can reduce the number of any types we got now.
I found a possible issue with the interaction: if you enable the error filter and you don't have errors (or any filter without results I guess), you end up not seeing anything. Even the breadcrumbs don't show up, so I felt a bit lost.

Another thing to consider, maybe in a future PR: as an alternative to passing the namespace and name (but still supporting both) to each kube resource, I think we could pass the object of the resource itself optionally, and render initially with it, until the item is gathered from the server. This would have the effect of e.g. preloading the last known state of that object, if you already have it from the graph or a list view, and thus it is immediate for the user, instead of them having to wait for the server responses.

@sniok sniok force-pushed the map-graph branch 3 times, most recently from 16ac9a9 to a955fa8 Compare October 25, 2024 14:11
@illume
Copy link
Copy Markdown
Contributor

illume commented Nov 1, 2024

In medium browser width, I can't really see anything because everything is very tiny.

image

@illume
Copy link
Copy Markdown
Contributor

illume commented Nov 1, 2024

Resizing the window from medium to large does not seem to trigger it to layout again, and everything remains tiny. Even though probably the layout should change with a wider browser.

If I refresh the browser it does seem to redo the layout to fit in the bigger window. Also, sometimes when I click the maximise button it does the layout properly with the bigger window (as apposed to dragging the window to resize or using window management shortcuts).

image

@illume
Copy link
Copy Markdown
Contributor

illume commented Nov 1, 2024

The nothing selected state for this looks strange:

image

@vyncent-t
Copy link
Copy Markdown
Contributor

works pretty solid for both web and app mode, currently have disabled gpu on so some icons may not be the exact same as current for me in app mode, been adding resources and looking at different sizes, seen a few of the same resize issues rene pointed to

@skoeva
Copy link
Copy Markdown
Contributor

skoeva commented Nov 1, 2024

^^ echoing the previous comments, seems to work pretty well but there are some concerns when resizing as well as performance-wise (but this may be unrelated, I have noticed the app can be a CPU hog sometimes lately)

@sniok
Copy link
Copy Markdown
Contributor Author

sniok commented Nov 4, 2024

I've finished reviewing.

The only issue blocking this is the text size/zoom issue. #2225 (comment)

I don't see this as a blocking issue. If the map is zoomed out to fit everything the items will be small, that's expected

@sniok
Copy link
Copy Markdown
Contributor Author

sniok commented Nov 4, 2024

Thanks everyone for the review, I've created an issue to keep track of all the non-blocking items here #2519

As we're nearing the release I'd like to minimize amount of big changes here, so let me know if anyone has objections to shipping this as is

@illume
Copy link
Copy Markdown
Contributor

illume commented Nov 4, 2024

I don't see this as a blocking issue. If the map is zoomed out to fit everything the items will be small, that's expected

It's an accessibility issue where most people can't read it.

@sniok sniok force-pushed the map-graph branch 4 times, most recently from b155d1f to cb55d2a Compare November 4, 2024 13:35
@sniok
Copy link
Copy Markdown
Contributor Author

sniok commented Nov 4, 2024

By default it now zooms in to 100%, with top-left origin if things don't fit or centered as before if things fit

image

Fit to view button will fit everything into view
100% button will zoom in to 100%, same as default

image

Max zoom is now also increased to 200%

Here are the changes
https://github.com/headlamp-k8s/headlamp/compare/1791c11944d00a1953fe60104781789588b88863..cb55d2a4bcbb92f117e35a37f29c6ad71b883ba8

@illume
Copy link
Copy Markdown
Contributor

illume commented Nov 5, 2024

Thanks for addressing that default size of text issue. Appreciate it.

I would like to say this is a really great accomplishment, and excellent work. 🎉

Especially by Olek (but also you Joaquim, plus Wilder, Grant, Ivelisse, and Chris for all the design work, as well as Vincent & Evangelos). With many weeks of reporting problems, it can feel unending (So, also thanks for putting up with all the wishes and issue reports in these last weeks/months).

sniok added 2 commits November 5, 2024 09:58
…meter

Some views want to use full width of the available page.
By default Layout Container will have a max width.
New isFullWidth parameter on the route removes max width

Signed-off-by: Oleksandr Dubenko <oldubenko@microsoft.com>
Signed-off-by: Oleksandr Dubenko <oldubenko@microsoft.com>
@sniok sniok force-pushed the map-graph branch 3 times, most recently from d03fb3a to 58102ee Compare November 5, 2024 09:35
sniok added 4 commits November 5, 2024 10:37
Introduce new resource Map view under /map route

Signed-off-by: Oleksandr Dubenko <oldubenko@microsoft.com>
Signed-off-by: Oleksandr Dubenko <oldubenko@microsoft.com>
Signed-off-by: Oleksandr Dubenko <oldubenko@microsoft.com>
Signed-off-by: Oleksandr Dubenko <oldubenko@microsoft.com>
@dosubot dosubot bot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 5, 2024
@joaquimrocha joaquimrocha merged commit 97daf03 into main Nov 5, 2024
@joaquimrocha joaquimrocha deleted the map-graph branch November 5, 2024 11:44
@sniok sniok mentioned this pull request Dec 3, 2024
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend Issues related to the frontend kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. resourceMap size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants