Adding an option to make drawer with horizontal text#1283
Adding an option to make drawer with horizontal text#1283Huy-Ngo wants to merge 5 commits intoedvin:masterfrom
Conversation
This is a temporary solution. The size of each button is fixed to 150px, which can be too wide or too narrow for someone else. Preferable solutions would be: - Make the drawer resizable and the items to fill the drawer - Make the drawer items' width to always be equal to the widest one Either solution sounds good to me, though for now I can't think of a way to implement either
SchweinchenFuntik
left a comment
SchweinchenFuntik
left a comment
There was a problem hiding this comment.
I have not yet understood what problem you are trying to solve, you have not referred to the issue
| @@ -321,7 +326,10 @@ class DrawerItem(val drawer: Drawer, title: ObservableValue<String?>? = null, ic | |||
| if (change.wasAdded()) { | |||
| change.addedSubList.asSequence() | |||
| .filter { VBox.getVgrow(it) == null } | |||
There was a problem hiding this comment.
no check HBox.getHgrow(it) == null
src/main/java/tornadofx/Drawer.kt
Outdated
| @@ -23,7 +24,7 @@ fun EventTarget.drawer( | |||
| op: Drawer.() -> Unit | |||
| ) = Drawer(side, multiselect, floatingContent).attachTo(this, op) | |||
There was a problem hiding this comment.
the old constructor is not used here
There was a problem hiding this comment.
append default value
| .filter { VBox.getVgrow(it) == null } | ||
| .forEach { VBox.setVgrow(it, Priority.ALWAYS) } | ||
| .forEach { | ||
| VBox.setVgrow(it, Priority.ALWAYS) |
There was a problem hiding this comment.
there is a potential increase in memory. You need to think about how to check which type of Panel is used
There was a problem hiding this comment.
Should I add a check for horizontal item here? In that case, I think I need to add a parameter for DrawerItem in order to check.
There was a problem hiding this comment.
might be worth redesigning ExpandedDrawerContentArea itself
| @@ -23,7 +24,7 @@ fun EventTarget.drawer( | |||
| op: Drawer.() -> Unit | |||
There was a problem hiding this comment.
append parameter horizontalItem
src/main/java/tornadofx/Drawer.kt
Outdated
| ) = Drawer(side, multiselect, floatingContent).attachTo(this, op) | ||
|
|
||
| class Drawer(side: Side, multiselect: Boolean, floatingContent: Boolean) : BorderPane() { | ||
| class Drawer(side: Side, multiselect: Boolean, floatingContent: Boolean, horizontalItem: Boolean) : BorderPane() { |
There was a problem hiding this comment.
it might be worth renaming. For example, by adding the prefix ʻis, it is possible to remove ʻItem.
I thought I already linked it by mentioning in the title, but for some reason GitHub didn't link it. I was trying to resolve #1267. I just edited my comment to show what I expect. |
|
Update on this? |
|
@edvin any idea when this comes into the upstream? I am eager to use it. :) |
Huy-Ngo
changed the title
|
Sorry, as I didn't have any experience writing a Kotlin library so I don't know how to make sure my patch works (It'd be nice if there's some automated CI tests/builds to verify it or an instruction in README). Also, there are some code issues as Funtik pointed out. |
|
@Huy-Ngo you can run tests directly in the IDE. Using task |
4 participants
Resolve #1267
This PR isn't done yet - I haven't written a Kotlin library before and moreover I'm not familiar with maven, so I don't know how to test and write test and was struggling to do so in the past month. Hopefully you can help me finishing this PR.