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

fix: add viewport for showing summary #1284

Closed
wants to merge 25 commits into from
Closed

fix: add viewport for showing summary #1284

wants to merge 25 commits into from

Conversation

divanshu-go
Copy link

@divanshu-go divanshu-go commented Oct 22, 2024 •
edited
Loading

fix: add viewport for showing project / workspace summary

Description

this PR adds support for scrollable project summary by adding viewport

  • This change requires a documentation update
  • I have made corresponding changes to the documentation

Related Issue(s)

Closes #1283
Closes #1298

Screenshots

image

Screencast.from.2024-10-23.12-17-49.mp4

Notes

Please add any relevant notes if necessary.

@divanshu-go divanshu-go requested a review from a team as a code owner October 22, 2024 18:43
bryans-go added 5 commits October 23, 2024 10:52
@divanshu-go
refactor code
af683e5 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
refactor code
51c2a51 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
calculate viewport size
02224db 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
add viewport help
bab0452 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
add viewport help
4634ef0 
Signed-off-by: bryans-go <[email protected]>
Tpuljak
Tpuljak requested changes Oct 23, 2024
View reviewed changes
pkg/views/workspace/create/summary.go Outdated Show resolved Hide resolved
pkg/views/workspace/create/summary.go Outdated Show resolved Hide resolved
pkg/views/workspace/create/summary.go Show resolved Hide resolved
bryans-go added 5 commits October 23, 2024 21:48
@divanshu-go
dynamic height-widthof viewport
09c053a 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
fix
fa9548d 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
refactor code
e71b9e0 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
refactor code
7e907a3 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
refactor code
f03f7c7 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go divanshu-go requested a review from Tpuljak October 24, 2024 07:28
Tpuljak
Tpuljak requested changes Oct 24, 2024
View reviewed changes
Copy link
Member

@Tpuljak Tpuljak left a comment

Choose a reason for hiding this comment

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

One minor comment to resolve. Nice work.

pkg/views/workspace/create/configuration.go Outdated Show resolved Hide resolved
bryans-go added 2 commits October 24, 2024 13:19
@divanshu-go
refactor code
e570217 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
refactor code
0372184 
Signed-off-by: bryans-go <[email protected]>
Tpuljak
Tpuljak previously approved these changes Oct 24, 2024
View reviewed changes
@Tpuljak Tpuljak requested a review from idagelic October 24, 2024 07:52
@divanshu-go
refactor code
2fd3211 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go

This comment was marked as abuse.

Sign in to view
bryans-go added 2 commits October 24, 2024 17:34
@divanshu-go
refactor code
f91bdb7 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go
refactor code
74a4b76 
Signed-off-by: bryans-go <[email protected]>
@divanshu-go

This comment was marked as abuse.

Sign in to view
@divanshu-go divanshu-go requested a review from Tpuljak October 24, 2024 17:18
Tpuljak
Tpuljak previously approved these changes Oct 25, 2024
View reviewed changes
idagelic
idagelic reviewed Oct 25, 2024
View reviewed changes
Copy link
Member

@idagelic idagelic left a comment

Choose a reason for hiding this comment

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

image

image

Is it possible to have the "SUMMARY" title inside the box? The layout/paddings at least seem a bit off

@divanshu-go
move title into summary content
cb6f5a9 
Signed-off-by: Divanshu Grover <[email protected]>
@divanshu-go

This comment was marked as abuse.

Sign in to view
idagelic
idagelic requested changes Oct 25, 2024
View reviewed changes
Copy link
Member

@idagelic idagelic left a comment

Choose a reason for hiding this comment

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

This looks good - left a minor suggestion

pkg/views/util/text_wrapper.go Outdated Show resolved Hide resolved
Divanshu Grover added 2 commits October 25, 2024 20:05
@divanshu-go

This comment was marked as abuse.

Sign in to view
idagelic
idagelic reviewed Oct 28, 2024
View reviewed changes
Copy link
Member

@idagelic idagelic left a comment

Choose a reason for hiding this comment

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

Yes, this seems to have fixed the env vars shuffling issue. Good work.

I have noticed some issues regarding displaying environment variables.
Here are a project-config add and a multiproject example:

Project config add - some env vars missing, scrolling does not do anything
image
image

Multi-project creation - sample-go project env vars are not shown
image
image

I see the env var display although better is not perfect on main either. Let me know if this is an easy fix as a part of this PR. Otherwise I will just open another issue for it

Divanshu Grover added 2 commits October 30, 2024 11:20
@divanshu-go
fix: arrangment of env vars
d3beb5a 
Signed-off-by: Divanshu Grover <[email protected]>
@divanshu-go
fix: arrangment of env vars
dd21919 
Signed-off-by: Divanshu Grover <[email protected]>
@divanshu-go

This comment was marked as abuse.

Sign in to view
Divanshu Grover added 2 commits October 30, 2024 15:29
@divanshu-go
fix: arrangment of env vars
2652663 
Signed-off-by: Divanshu Grover <[email protected]>
@divanshu-go
fix: arrangment of env vars
a4f878e 
Signed-off-by: Divanshu Grover <[email protected]>
idagelic
idagelic requested changes Oct 31, 2024
View reviewed changes
Copy link
Member

@idagelic idagelic left a comment

Choose a reason for hiding this comment

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

image

All the env vars seem to appear correctly now. However, the second issue (second set of screenshots) still exists - I added two env var entries for Project 2 but they aren't shown. Does the multi-project variant work on your end?

@Tpuljak
Copy link
Member

Tpuljak commented Nov 14, 2024

@divanshu-go any updates for this?

@daytonaBot
Copy link
Collaborator

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 14 days). It will be closed if no further activity occurs. Thank you for your contribution.

@Tpuljak Tpuljak closed this Nov 29, 2024
@mojafa mojafa mentioned this pull request Dec 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@idagelic idagelic idagelic requested changes

@Tpuljak Tpuljak Awaiting requested review from Tpuljak

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

Successfully merging this pull request may close these issues.

Summary view env var weirdness non-scrollable TUI on multiple project summary view
4 participants
@divanshu-go @Tpuljak @daytonaBot @idagelic