Skip to content

Conversation

@pithecuse527
Copy link
Contributor

Description

Change the order of the height and width displayed in WindowTooSmallModel

image

Related Issue

#28

Changed Made

  • Reorder the WindowTooSmallModel Width and Height to display properly
  • Added test cases for the WindowTooSmallModel

@ajayd-san
Copy link
Owner

Hey,

One small change, it would be helpful to add the minimum dimensions required as well.

Can you add something like Min Width: 65, Min Height: 25 between those two lines?

Thanks

@pithecuse527 pithecuse527 force-pushed the fix/change-height-width-order-in-windowtoosmallmodel#28 branch 3 times, most recently from 809fced to a8dc96b Compare July 22, 2024 13:44
@pithecuse527
Copy link
Contributor Author

Min. requirements added!

image

@pithecuse527 pithecuse527 force-pushed the fix/change-height-width-order-in-windowtoosmallmodel#28 branch from a8dc96b to eb8af6b Compare July 22, 2024 13:46
@ajayd-san
Copy link
Owner

ajayd-san commented Jul 22, 2024

I think it would be better to come up with something concise:

Minimum dimensions needed - Width: 65, Height: 25

Please change to this instead.

(ISTG this is the last change! :) )

@ajayd-san ajayd-san changed the base branch from main to v1.4 July 22, 2024 17:21
@pithecuse527 pithecuse527 force-pushed the fix/change-height-width-order-in-windowtoosmallmodel#28 branch from eb8af6b to 2d3754d Compare July 22, 2024 18:41
@pithecuse527
Copy link
Contributor Author

Done!

image

@ajayd-san
Copy link
Owner

Perfect!!

Thanks for your help @pithecuse527

@ajayd-san ajayd-san merged commit 87f2da1 into ajayd-san:v1.4 Jul 22, 2024
@pithecuse527
Copy link
Contributor Author

BTW,
It is advisable to use constant variables for minimum values, such as the minimum height for window size or the threshold for toggling the info box. If needed, we can address this in another issue.

@pithecuse527 pithecuse527 deleted the fix/change-height-width-order-in-windowtoosmallmodel#28 branch July 22, 2024 18:52
@ajayd-san
Copy link
Owner

ajayd-san commented Jul 22, 2024

Yes, I totally agree.

If you can address that, that would be great.

  • specify min heights and widths need for tui (in consts)
  • specify min heights and widths needed for infobox (in consts)

I already have ratios in constants in mainModel.go, so that is not needed:

// dimension ratios for infobox
const infoBoxWidthRatio = 0.55
const infoBoxHeightRatio = 0.6

Create a new issue and we can discuss more over there if required. Thanks!

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.

2 participants