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

Improving Home Button function #4281

Merged
merged 5 commits into from
Jan 24, 2025
Merged

Conversation

omsuneri
Copy link
Contributor

@omsuneri omsuneri commented Jan 17, 2025

@walterbender Now the user can toggle between two different modes of home button
on 1st click arrangement in Rows
on 2nd click arrangement in columns
https://github.com/user-attachments/assets/1cac3cfd-7a5e-4644-bc09-eb335899bdb8

@omsuneri
Copy link
Contributor Author

@walterbender i had changed the home button method now it creates 2 column of block arrangement review and request if changes are required

@omsuneri
Copy link
Contributor Author

@walterbender after merge of this changes i ll make the triggering of home button whenever the png is downloaded

@walterbender
Copy link
Member

I like where this is headed. But maybe the number of columns is based on the screen width? (And the width of the stacks themselves?)

@omsuneri
Copy link
Contributor Author

Okay @walterbender so you want that multiple columns according to the stack width

@omsuneri
Copy link
Contributor Author

@walterbender but it seems to be difficult as the stack width depends on interconnection of blocks and they actually overlap somewhere

@walterbender
Copy link
Member

I agree--it is not easy to figure out how wide a stack is. In your implementation of 2 columns, you are making an assumption about the width. (It is not the end of the world if that assumption fails.) I guess where I am heading is if you have a phone, you prob. only want 1 column. If you have a desktop, maybe 3 or 4.

@omsuneri
Copy link
Contributor Author

Yaaa 😅 I just make a simple assumption but must update it to a logical approach @walterbender I ll be doing the requested updates very soon

@omsuneri
Copy link
Contributor Author

@walterbender i tried the 3 column approach on the desktop but there is still overlapping so can we have a 1 column for mobile and 2 for desktop
Screenshot 2025-01-21 at 9 03 18 AM
what would you say should i go with the 3 column approach

@walterbender
Copy link
Member

1 column for "mobile" for sure. But since it is possible to make arbitrarily wide stacks, there is no multi-column solution that will guarantee no overlap unless we calculate the width of each column, something we decided against. So the question is, what is the minimum column width we want to support? And let's then take that minimum and divide the screen width by it and thus determine the number of columns.

@omsuneri
Copy link
Contributor Author

@walterbender okay i understand the approach will set 1 for mobile and for the no. of column what would you say the minimum column width i guess as per my observation 3 column for a desktop is fine so according to that should i approximate the min column width

@walterbender
Copy link
Member

I don't know what size desktop you are testing with, so it is hard to assess.

@omsuneri
Copy link
Contributor Author

@walterbender I m on Mac air 13.3 inches I guess

@walterbender
Copy link
Member

But how many pixels wide?

@omsuneri
Copy link
Contributor Author

@walterbender 1440 x 932

@walterbender
Copy link
Member

Not sure how accurate this is:

Device Type Resolution Range (Width x Height) Typical Orientation
Desktop 1024 x 768 – 5120 x 2880 Landscape (primary), Portrait (supported)
Mobile 320 x 480 – 1440 x 3200 Portrait (primary), Landscape
Tablet 768 x 1024 – 2560 x 1600 Portrait (common), Landscape

But it would suggest a column width of many 600 pixels? What ever we chose, we should document with a comment in the code.

@omsuneri
Copy link
Contributor Author

@walterbender okay so basically the minimun column width hereby is the 600 px and then this goes on with the screen width

@walterbender
Copy link
Member

So ncols = int(screenwidth / 600)

@omsuneri
Copy link
Contributor Author

@walterbender I got this point will be updating the function

@pikurasa
Copy link
Collaborator

@omsuneri I tested this just now, and, for some projects, it will require a lot of vertical scrolling.

Please test "Rainbow Connection" in examples.

For larger projects, it would be nice if the blocks were not spaced so far apart vertically.

@omsuneri
Copy link
Contributor Author

@walterbender I had changed the algo for selecting the number of columns for a particular screen size dynamically also without the maximum number of column it can be any depends on the screen width also i kept the minimum column width to be 400px its seems something better than 600px to me
please review this

@omsuneri
Copy link
Contributor Author

@omsuneri I tested this just now, and, for some projects, it will require a lot of vertical scrolling.

Please test "Rainbow Connection" in examples.

For larger projects, it would be nice if the blocks were not spaced so far apart vertically.

@pikurasa I found that now i had changed the number of columns to be dynamic depends on the screen width you can check the latest commit here and request if any changed further required

@pikurasa
Copy link
Collaborator

Please test "Rainbow Connection" in examples.

For larger projects, it would be nice if the blocks were not spaced so far apart vertically.

@omsuneri this is what I experience with a project, like Rainbow Connection, that has many Action Blocks (tested from HEAD is now at 3f9ca61 updating the number of columns of home button):

home-function-test.webm

I don't think we want that, do we??

@omsuneri
Copy link
Contributor Author

@pikurasa can you jsut try expanding blocks

@pikurasa
Copy link
Collaborator

@pikurasa can you jsut try expanding blocks

Yes, but how does that help the user experience?

@omsuneri
Copy link
Contributor Author

@pikurasa actually earlier the home button is just overlapping the blocks and seems difficult to understand the blocks connection i totally agree with your point for the large projects but to convert to .svg or .png of the blocks connections we need to get rid of overlapping of blocks thats why i improved this button architecture

@omsuneri
Copy link
Contributor Author

@pikurasa @walterbender what did we desired to be done in order to improve the structure with large projects

@pikurasa
Copy link
Collaborator

I think this arrangement makes total sense -- and for both large and small projects -- for SVG and PNG export.

However, I remain skeptical about its usefulness for the Home Button. A user only ever can concentrate on a few expanded Actions at a time. Meanwhile, they can sort of see the structure of the code by seeing the collapsed ones on their screen with minimal scrolling.

Why not just use this for SVG/PNG export?

@omsuneri
Copy link
Contributor Author

I think this arrangement makes total sense -- and for both large and small projects -- for SVG and PNG export.

However, I remain skeptical about its usefulness for the Home Button. A user only ever can concentrate on a few expanded Actions at a time. Meanwhile, they can sort of see the structure of the code by seeing the collapsed ones on their screen with minimal scrolling.

Why not just use this for SVG/PNG export?

@pikurasa yaa this can be done like when a user export the svg or png this function get triggers automatically instead of refactoring the Home button @walterbender what would you say on this

@walterbender
Copy link
Member

I wonder: What if we have the Home button toggle between two modes that (1) maintains a small fixed vertical distance between stacks (like we have a fixed distance horizontally) and (2) removes potential overlap between stacks. Clicking on the Home button would jump back and forth between the two states.

Or we detect if a stack is collapsed and use the collapsed height instead of the expanded height.

But I do think the current Home configuration crowds things too much.

@omsuneri
Copy link
Contributor Author

@walterbender so the home button on single must react with the normal arrangement with some vertical distance set and then another click makes the block arrangement looks like what we have did now it's that looks fine

@omsuneri
Copy link
Contributor Author

@walterbender can we do like what the home button is doing previously for single click and the next click for what we have in this pr

@walterbender
Copy link
Member

Let's make it toggle between the two modes, but I think we should also space out (horizontally and veritically) the current Home behavior a bit. It is too tight. Maybe 20%?

@omsuneri
Copy link
Contributor Author

@walterbender so I must increase the spaces between the two row and columns and will be making it toggle to switch between two states of the home 1. The previous what we have and 2. The current state in this pr with increased spacing.

@pikurasa
Copy link
Collaborator

Let's make it toggle between the two modes, but I think we should also space out (horizontally and veritically) the current Home behavior a bit. It is too tight. Maybe 20%?

I agree. Let's try that approach.

@omsuneri
Copy link
Contributor Author

@pikurasa will be making it soon

@omsuneri
Copy link
Contributor Author

@pikurasa @walterbender i had completed the creation of home button toggle between two modes kindly review this again and request if any further changes are required
https://github.com/user-attachments/assets/7d62432d-842c-44d1-bbc3-4616e130c81b

@pikurasa
Copy link
Collaborator

@pikurasa @walterbender i had completed the creation of home button toggle between two modes kindly review this again and request if any further changes are required https://github.com/user-attachments/assets/7d62432d-842c-44d1-bbc3-4616e130c81b

What I see in the video looks good, but what I understood from Walter's "I think we should also space out (horizontally and veritically) the current Home behavior a bit. It is too tight. Maybe 20%?" comment is that we should space the blocks a bit more (in the mode that is similar to our current home functionality).

Based on that, it still seems too tight. Perhaps space them, at least vertically, a bit more than they are atm. (Try a few values until you find something that seems to work well.)

@omsuneri
Copy link
Contributor Author

@pikurasa @walterbender i had completed the creation of home button toggle between two modes kindly review this again and request if any further changes are required https://github.com/user-attachments/assets/7d62432d-842c-44d1-bbc3-4616e130c81b

What I see in the video looks good, but what I understood from Walter's "I think we should also space out (horizontally and veritically) the current Home behavior a bit. It is too tight. Maybe 20%?" comment is that we should space the blocks a bit more (in the mode that is similar to our current home functionality).

Based on that, it still seems too tight. Perhaps space them, at least vertically, a bit more than they are atm. (Try a few values until you find something that seems to work well.)

So @walterbender @pikurasa i think you are asking the 1st old case to space more vertically cause i had increased the spacing by 20% in the new case and now the blocks are somewhat apart but if you want i can increase it more also for the old case if we want i can aslo increase its spacing too..

@walterbender
Copy link
Member

I really like this new behavior.

Please: (1) remove spaces from blank lines; (2) I am seeing one weird artifact: the spacing between columns is mismatch Not sure why. (Observe in the image the space between the 3rd and 4th stacks, counting from the left.)

Screenshot From 2025-01-24 12-33-13

@omsuneri
Copy link
Contributor Author

Add your comment here...

Markdown is supported

@walterbender i havent changed anything with the old version of home button so you are in that state i can do that to make the spacing equal

@walterbender
Copy link
Member

I figured out what is going on with the spacing. Somehow in the project I was looking at, a stray "hiddennoflow" block was in the mix. Not sure how that block got there, but its presence disrupts the spacing. I think it is not something we should fix.

@omsuneri
Copy link
Contributor Author

I figured out what is going on with the spacing. Somehow in the project I was looking at, a stray "hiddennoflow" block was in the mix. Not sure how that block got there, but its presence disrupts the spacing. I think it is not something we should fix.

@walterbender Okay so i ll be not be changing the spacing issue you mentioned earlier so it is merge ready or any another changes we need right now

@walterbender walterbender merged commit ba43b79 into sugarlabs:master Jan 24, 2025
4 checks passed
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.

3 participants