VisUI support about ktx HOT 13 CLOSED

Added support for simplest widgets.

I don't know about unit testing, I tried with mocking needed stuff but it's pain to get it working, e.g. I needed to mock Gdx.files because Cell uses it for something, go figure. If it would be ok I would prefer to use something like gdx-testing but with standard LWJGL backend, headless won't do since loading skin requires loading texture. Or we can just ignore unit testing for this module and I can just make manual test app like in case of VisUI.

from ktx.

Yeah, LibGDX can be a pain to test due to its singletons and statics. 100% test coverage would be nice, as we would immediately know if something breaks after LibGDX version change, but I understand that anything involving the GUI can be problematic. Using LWJGL backend - or any other desktop backend, for that matter - is perfectly OK. Just make sure to init LWJGL statics once before all tests to speed things up.

I suggest making a simple abstract test for WidgetFactory. Mock skin (or load it with LWJGL backend, whatever works for you best), make sure that the factories return non-null actors of the expected type or a properly wrapped actor. Check if the actor is in the group. I know that this basically tests constructors and Group/Table implementations, but there's not much to test in the first place - this is basically syntax sugar for Scene2D API, after all.

from ktx.

Ok, added unit tests, this is probably overkill because because it actually tests every factory method of every widget (577 tests total) but hey 100% coverage without that much work. Returned type isn't tested because unchecked casts would fail anyway in that case which would show up in tests.

I removed excess constructors from KWidgets, after all those widgets are only expected to be created from within WidgetFactory and hidden from user as much as possible.

Btw, I would be in favor of excluding (Vis)Dialog from builders, it has its own button , text methods which would lead to confusion, users just should use Window for dialogs.

from ktx.

Since you intend on removing VisDialog from VisUI, I think it's OK to omit it completely. Especially since Kotlin is much better at adding listeners than pure Java (pre-8), so clunky Dialog "result" approach is not necessary.

from ktx.

@czyzby need some input

• MessageToast not really needed, can be used directly
• TabbedPane is always splited into two widgets: tabs pane and tab content. I think we shouldn't add any method for this and make user create it manually. Adding standard function to WidgetFactory would be confusing: what would be exactly added? TabbedPane tabs table? What is returned? TabbedPane itself or table cell, how users can customize cell if TabbedPane is returned and so on. I think it's just better to left managing it manually.
• DragPane - what features of DragPane should be supported?
• FormValidator - any idea how to integrate FormValidator into builders? This would work out the box:

table { 
      validator.fileExists(validatableTextField("").actor, "")
 }

Do we need different way?

• ButtonGroup isn't a widget, buttons must be added manually to it using it's add method. I was thinking about adding Button extension method addToButtonGroup(ButtonGroup) or it can be used like FormValidator which would require no extra work.

Working on menus now, this looks really sweet, especially when compared to Java way:

menuBar {
  menu("File") {
    menuItem("Open")
    menuItem("Edit") {
      subMenu {
        menuItem("SubMenuItem")
      }
    }
  }
}

from ktx.

• If you consider any API to be "Kotlin-like" or overly complex as a builder - feel free to not add it at all.
• In LML, I used to add a so-called "content" Table to the table with tabs. Content table was always under (in case of horizontal tabs) or on the right (vertical tabs), which obviously is not the most flexible solution. I guess you could add a tab method to TabbedPane, which would allow to specify the tab button, container actor and where exactly the content is added.
• It would be nice if DragPane was a one-child group similar to Container - it would throw an exception if its child was not a Group. I expect its usage to look similar to this:

dragPane {
  setDraggable(...)
  horizontalFlowGroup {
     ...
  }
}

• Again, in LML I just created an extra Table extension which manages a validator. Each time a validatable actor was added to the table, it was also registered in the validator. See VisFormTable.
• ButtonGroup would also benefit from a wrapper.

from ktx.

DragPane

dragPane {
  setDraggable(...)
  horizontalFlowGroup {
     ...
  }
}

I assumed actors have to be added to DragPane and not its WidgetGroup. I expected something like this:

dragPane(HorizontalFlowGroup()) {
  setDraggable(...)
  label()
  //...
}

If widgets have to be added to DragPane directly and we want to have API like in first example then it would require to make new class for each supported WidgetGroup which would delegate adding actors to DragPane. Not worth it imo.

FormValidator

In LML you pre-add validators to fields before adding them to VisFormTable, in ktx-vis it would require to write extension methods for adding validators thus binding us to VisUI API,
I see those solutions:

• Works pretty much out of the box, one disadvantage is need for .actor when table is used.

table {
  validator {
    notEmpty(validatableTextField("Example").expand().actor, "Msg")
  }
}

verticalGroup {
  validator {
    notEmpty(validatableTextField("Example"), "Msg")
  }
}

• Same as above but I add methods that also takes Cell<Actor>, would remove need for .actor. In case of some API change missing methods can be used with .actor.
• Similar to LML:

table {
  validator {
    validatableTextField("Example").expand().notEmpty("ErrorMsg")
  }
}

Extension methods adding validators for ValidatableTextField (and checkbox). This has serious disadvantage imo, notice that FormValidator needs FormInputValidator (which also provides error messages). Extension methods for all validators would allow to add validators that aren't FormInputValidator eg. IntegerValidator. Some validators would have error messages, some wouldn't, and if you use them outside validator then error messages would be ignored anyways, it would be rather confusing. And of course it adds need to track changes in VisUI.

I think first option is best here but maybe I'm missing something. What do you think @czyzby ?

from ktx.

DragPane

Yeah, that looks even better and is definitely less complex. Go with the group actor passed to the dragPane method.

FormValidator

Works pretty much out of the box, one disadvantage is need for .actor when table is used.

Are you sure? notEmpty extension method consuming Cell<ValidatableTextField> could be added to validator to automatically extract the actor from the cell.

Your first approach seems all right. Too many extension methods do make the code a little bit more fluent, but also less readable in the end imho.

from ktx.

Are you sure? notEmpty extension method consuming Cell could be added to validator to automatically extract the actor from the cell.

Yeah that's what I wrote in second solution:

Same as above but I add methods that also takes Cell, would remove need for .actor.

But I think I will leave it as is, I don't like the need to keep it up to such level with VisUI API.

By the way should I write README.md?

from ktx.

Oh, I missed that part. Cell-consuming extension would make it a little bit more readable, but I leave it up to you.

Yeah, README.md with a similar structure to the other modules would be nice.

from ktx.

Alright finally added TabbedPane support, example:

table {
  tabbedPane("vertical") {
    tab("Tab1") {
      label("Inside tab 1")
    }
    tab("Tab2") {
      label("Inside tab 2")
    }
    tab("Tab3") {
      label("Inside tab 3")
    }

    addTabContentsTo(table().grow())
    //OR addTabContentsTo(container<Table>().grow())

    switchTab(0)
  }.cell.growY()
}

Or when you want to add tab content somewhere else:

table {
  val (pane, cell) = tabbedPane {
    tab("Tab1") {
      label("Inside tab 1")
    }
    tab("Tab2") {
      label("Inside tab 2")
    }
    tab("Tab3") {
      label("Inside tab 3")
    }
    switchTab(0)
  }
  cell.growX().row()
  label("Some content in between").growX().row()
  pane.addTabContentsTo(table())
}

from ktx.

@czyzby Added README, fell free to close or critique. ;)

from ktx.

In most modules I tried to list and explain every added feature, but it's needless to say that ktx-vis is huge and listing every method could be pointless.

It's alright, I'll post some small fixes (like typos, missing commas/dots and so on) and close the issue. Nice job.

from ktx.