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

Add new button label positions along with new vertical tasklist mode when labelpos is set to top or bottom #615

Merged
merged 10 commits into from
Jul 27, 2024

Conversation

purpasmart96
Copy link
Contributor

This is a WIP prototype of vertical buttons. They are now utilized for the tasklist when the tray is vertical. I'm not quite yet satisfied with it yet since there are still some issues with resizing and the text disappearing too early when the task icon is too big. But I plan to address those issues.

It will currently try to center the text if the text width is less than or equal to the icon width, otherwise, it will have the text on the left.

To avoid too many changes I just made another Button function that handles drawing instead of adding new logic to the existing DrawButton function.

I did take out some of the existing logic that was in DrawButton and move them into their own functions but I only did that for some parts of it since the parts that use preprocessor macros didn't end up working for me. So I just left them as is.

I posted my results in an existing issue over at #457
But I don't think too many people noticed it so I'm posting it here for further feedback/ideas.

Screenshot_20240627_152205

FYI: The User defined height attribute for the tasklist is ignored for now.

@joewing
Copy link
Owner

joewing commented Jul 7, 2024

I think this should probably be a configuration option on the tray or maybe it should use the tray width to decide what to do. I suspect there are users who want trays to show up with the icon and text as they would appear in a menu. Otherwise I think this looks good.

@purpasmart96
Copy link
Contributor Author

I think this should probably be a configuration option on the tray or maybe it should use the tray width to decide what to do. I suspect there are users who want trays to show up with the icon and text as they would appear in a menu.

Yeah, I was thinking of having this as an alternative tray mode you can toggle on, otherwise it will use the original logic for the tray when vertical.

…st. By default it is set to false and will use the previous vertical tasklist logic. Also did some minor refactoring of the ComputeItemSize function.
@purpasmart96
Copy link
Contributor Author

purpasmart96 commented Jul 10, 2024

@joewing
Alright, I added "altvertlist" attribute to toggle on or off the new vertical tasklist in the TaskListStyle.
Do You think this name is fine? And what order would you like this attribute to be in TaskListStyle? Right now I have it positioned after "list".

By default, it is set to false and will use the previous vertical tasklist logic.

I also did some minor refactoring of the ComputeItemSize function.

It already uses the tray width to automatically calculate the tasklist item size. Now, since this is a vertical tray things get swapped around since the actual width is the tray height and the real height is the tray width if that makes any sense.
So I end up reusing the "maxwidth" attribute from TaskList as the real max height.

Basically, everything is automatically calculated the same way it's done normally when the tray is horizontal, but with some height and width swapped.

I'm going to try to adjust the vertical button so it prioritizes the label when it's enabled so it doesn't disappear so early when room starts running out in the tray.

@joewing
Copy link
Owner

joewing commented Jul 14, 2024

For the configuration, it might make it easier to use if it was more explicit... Maybe something like

<TaskListStyle labelpos="right" ...>

and

<TaskListStyle labelpos="bottom" ...>

That way there is also an obvious path to placing the label above or to the left.

@purpasmart96
Copy link
Contributor Author

Yeah, that looks way more clear.

@purpasmart96
Copy link
Contributor Author

purpasmart96 commented Jul 16, 2024

@joewing
Do you think labelpos should be part of TaskListStyle or part of from the tray?
Edit: Yeah I think having the label position as part of <TaskList/> makes sense. Having it defined in the TaskListStyle means that it will apply for all trays.

@joewing
Copy link
Owner

joewing commented Jul 17, 2024

Yeah, having it as part of the tray or TaskList probably makes more sense. It's more a layout thing than a style thing anyway.

@purpasmart96
Copy link
Contributor Author

purpasmart96 commented Jul 22, 2024

@joewing
Well, I ended up doing that and much more. I added button label positions. The button now has 3 different positions. right, top, and bottom.

You can now also have buttons of different styles regardless of tray orientation.

bottom labels on horizontal tray.

horz_bottom_scrot

bottom labels on vertical tray.

vertical_bottom_scrot

top labels on horizontal tray.

horz_top_scrot

top labels on vertical tray.

vertical_top_scrot

By default the button label position is LABEL_POSITION_RIGHT and everything should stay the same by default.

@purpasmart96
Copy link
Contributor Author

I might rename the PR to reflect the new changes.

@purpasmart96 purpasmart96 changed the title WIP: Vertical tasklist with vertical buttons Add new button label positions along with new vertical tasklist mode when labelpos is set to top or bottom Jul 23, 2024
@purpasmart96
Copy link
Contributor Author

purpasmart96 commented Jul 23, 2024

@joewing
Should be ready to go. I could also further abstract DrawButton if you want.

I can put the logic for drawing the border and background into their separate functions.

@joewing
Copy link
Owner

joewing commented Jul 26, 2024

Looks good, thanks! I think the only thing left to do would be to document the new feature in jwm.1.in.

@purpasmart96
Copy link
Contributor Author

Looks good, thanks! I think the only thing left to do would be to document the new feature in jwm.1.in.
Ahh ok, I will take care of that real quick!

@purpasmart96
Copy link
Contributor Author

@joewing
Alright. It should be ready to go!

@joewing
Copy link
Owner

joewing commented Jul 27, 2024

Thank you!

@joewing joewing merged commit 9319813 into joewing:master Jul 27, 2024
@purpasmart96
Copy link
Contributor Author

Thank you!

Glad to get this merged!

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