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

MouseEvents should contain the relative position inside its component #335

Open
Baret opened this issue Nov 2, 2020 · 5 comments
Open
Labels
enhancement Enhances existing functionality without introducing new features

Comments

@Baret
Copy link
Collaborator

Baret commented Nov 2, 2020

Describe the bug
Currently when adding a mouse listener to a component you can get the position of the click (or movement) from it. But this position is absolute. This means it is independent of the component's position. This becomes a problem when you add a handler that has no idea of where the component is located on the screen.

To Reproduce

import org.hexworks.zircon.api.ColorThemes
import org.hexworks.zircon.api.ComponentDecorations
import org.hexworks.zircon.api.Components
import org.hexworks.zircon.api.SwingApplications
import org.hexworks.zircon.api.application.AppConfig
import org.hexworks.zircon.api.data.Position
import org.hexworks.zircon.api.data.Size
import org.hexworks.zircon.api.extensions.toScreen
import org.hexworks.zircon.api.uievent.MouseEventType
import org.hexworks.zircon.api.uievent.Processed

private val WINDOW_SIZE = Size.create(60, 30)

private val panel = Components
        .panel()
        .withPosition(8, 5)
        .withSize(42, 20)
        .withDecorations(ComponentDecorations.box())
        .build()

private val clickLabel = Components
        .label()
        .withPosition(Position.zero())
        .withSize(WINDOW_SIZE.width, 1)
        .build()

private val clickLabelAbsolute = Components
        .label()
        .withPosition(Position.bottomLeftOf(clickLabel))
        .withSize(WINDOW_SIZE.width, 1)
        .build()

private val infoLabel = Components
        .label()
        .withPosition(Position.bottomLeftOf(clickLabelAbsolute))
        .withSize(WINDOW_SIZE.width - 1, 1)
        .build()

fun main() {
    val screen = SwingApplications
            .startTileGrid(AppConfig
                    .newBuilder()
                    .withSize(WINDOW_SIZE)
                    .withTitle("Mouse events")
                    .build())
            .toScreen()
    with(screen) {
        addComponents(
                panel,
                clickLabel,
                clickLabelAbsolute,
                infoLabel
        )
        theme = ColorThemes.war()
        display()
    }

    panel.handleMouseEvents(MouseEventType.MOUSE_CLICKED) { event, phase ->
        // Imagine that panel.absolutePosition is unknown here...
        val absolutePosition = event.position
        val position = absolutePosition - panel.absolutePosition
        clickLabel.textProperty.updateValue("You clicked       ${position.x},${position.y} in the panel")
        clickLabelAbsolute.textProperty.updateValue("Event position is ${absolutePosition.x},${absolutePosition.y}")
        Processed
    }

    infoLabel.textProperty.updateValue("Abs. position of panel: ${panel.absolutePosition}")

}

Expected behavior
When handling a mouse event I would expect to either get the event.position inside the component that handles the event. Or have two fields in the event: event.absolutePosition and event.position.

@adam-arold
Copy link
Member

The mouse listener works with any Zircon object, not just Components, that's why it doesn't have a relative position. You bring up a valid point though. Can you think of a way we could augment MouseEvent without breaking the API? I'm gonna add this to the backlog.

@adam-arold adam-arold added the enhancement Enhances existing functionality without introducing new features label Nov 2, 2020
@Baret
Copy link
Collaborator Author

Baret commented Nov 3, 2020

Without deeper checks of where mouse events are created and consumed...

Without breaking the API? Probably by creating a new class that gets new special treatment (new handleXEvent() methods and so on):

/**
 * A mouse event triggered by a [Component].
 */
data class MouseComponentEvent(
        override val type: MouseEventType,
        val button: Int,
        /**
         * Position of this event relative to the top left corner of the tile grid/screen.
         */
        val position: Position,
        /**
         * The [Component] that triggered this event.
         */
        val source: Component
) : UIEvent {
    /**
     * The position of this event inside the [source] component. This position is relative to
     * the top left corner of [source]'s absolutePosition.
     *
     * @see Component.absolutePosition
     */
    val positionInSource: Position
        get() = position - source.absolutePosition
}

fun MouseEvent.toMouseComponentEvent(source: Component) =
        MouseComponentEvent(
                type,
                button,
                position,
                source
        )

But with breaking the API? Just a quick mockup of which I acutally don't know if it woud break the API because of the default value:

data class MouseEvent<T: Any>(
        override val type: MouseEventType,
        val button: Int,
        val position: Position,
        val source: T? = null
) : UIEvent

val MouseEvent<Component>.positionInComponent: Position
    get() = position - (source?.absolutePosition ?: Position.zero())

@adam-arold
Copy link
Member

This looks like a good idea, I dig it. I'm gonna take a look after the next release if this can be implemented. In the meantime, you are welcome to give it a try if you're interested.

@Baret
Copy link
Collaborator Author

Baret commented Nov 3, 2020

Damn, when adding a generic type parameter to MouseEvent we would need to add it in almost every place. Usually this would probably simply result in changing MouseEvent to MouseEvent<*>.
I will see what else changes with that try...

@adam-arold
Copy link
Member

Oh, yeah...generics is cancer in this case...once I refactored Component to be generic just to undo it after I was done because it was so horrifying...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances existing functionality without introducing new features
Projects
None yet
Development

No branches or pull requests

2 participants