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

x/exp/shiny: no API to change a window size #13339

Open
sbinet opened this issue Nov 20, 2015 · 13 comments
Open

x/exp/shiny: no API to change a window size #13339

sbinet opened this issue Nov 20, 2015 · 13 comments
Milestone

Comments

@sbinet
Copy link
Member

@sbinet sbinet commented Nov 20, 2015

it seems there is no way to programmatically change a screen.Window size.

currently, a screen.Window size can only be changed from outside the program (e.g. using the mouse).

I'd propose to extend screen.Window as such:

type Window interface {
  // ...

    // Resize resizes the window.
    Resize(size image.Point)

    // Size returns the size of the window.
    Size() image.Point
}
@dskinner
Copy link
Member

@dskinner dskinner commented Nov 20, 2015

Given there is the type NewWindowOptions here https://github.com/golang/exp/blob/master/shiny/screen/screen.go#L145

What if that type were simply WindowOptions and type Window interface declared something along the lines of SetOptions(opt WindowOptions), that way it can remain flexible with time as more options are added.

@crawshaw
Copy link
Contributor

@crawshaw crawshaw commented Nov 20, 2015

cc @nigeltao

First off, adding a Size method will have to wait until https://golang.org/cl/17055 is done and is more about how much state the Window object should track and make available. I'd like to punt that to another issue if you don't mind.

As for Resize, having a way to do it is reasonable. But there are several other related options and I'd like to think through how those work in an API. For example:

  • move
  • minimize / maximize
  • fullscreen

Do all of these have to have their own methods? Can (or should?) they be combined into an Options struct? Additionally, there's going to be a lot of overlap between these features and NewWindowOptions. Is there any overlap that can be reused?

@dskinner
Copy link
Member

@dskinner dskinner commented Nov 20, 2015

The first issue that comes to mind with a combined options struct is what zero values mean. If I have a window specifically sized at 400x300 and I simply want to update the window transparency, I might expect win.SetOptions(WindowOptions{Transparency: 0xCC}) to work, yet this might trigger the driver to change the window size since width/height are zero in new options. Something to think about.

@sbinet
Copy link
Member Author

@sbinet sbinet commented Nov 20, 2015

for this, the functional-options idiom is probably best:
http://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis

// ----------------
package screen
var (
  WindowSize func(Window)
  WindowTransparency func(Window)
)

// -----------------
package mydriver

func (s *screenImpl) NewWindow(opts ...func(w screen.Window)) (screen.Window, error) {
  // ...
}

func windowSize(width, height int) func(w screen.Window) {
  return func(w screen.Window) {
     xw := w.(*windowImpl)
     xw.width = width
     xw.height = height
  }
}

func windowTransparency(v uint16) func(w screen.Window) {
  return func(w screen.Window) {
     xw := w.(*windowImpl)
     xw.setTransparency(v)
  }
}

func init() {
  screen.WindowTransparency = windowTransparency
  screen.WindowSize = windowSize
}

// -------------------
package main

func foo(s screen.Screen) {
  w, err := s.NewWindow(
       screen.WindowTransparency(0xCC),
       screen.WindowSize(600,600),
  )
  //...
}

(except "of course" when screen doesn't know anything about xyzdriver.windowImpl, so gymnastics are needed...)

@crawshaw
Copy link
Contributor

@crawshaw crawshaw commented Nov 20, 2015

@sbinet I believe a simple version of the gymnastics required for the driver packages would be to make each option a data type instead of a function;

func NewWindow(opts ...WindowOption) { /* ... */ }

type WindowOption interface {
        windowOption()
}

type WindowSize image.Point

func (WindowSize) windowOption() {}

type WindowPosition image.Point

func (WindowPosition) windowOption() {}

func f() {
        // ...
        NewWindow(
                WindowPosition(image.Pt(10, 20)),
                WindowSize(image.Pt(300, 300)),
        )
}

Each driver can switch on the concrete type of each WindowOption and do what it sees fit with each value. WDYT?

@sbinet
Copy link
Member Author

@sbinet sbinet commented Nov 20, 2015

SGTM

@rogpeppe
Copy link
Contributor

@rogpeppe rogpeppe commented Nov 21, 2015

Drive-by comment:
I have to say I'm not a huge fan of either of the above approaches, although I understand the motivation. A simple struct (with pointers where necessary) always strikes me as much more clear even if it might be a little more awkward to use sometimes.

@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented Nov 23, 2015

As @crawshaw hinted at, I'd like to avoid incrementally adding a dozen odd SetThis and SetThat methods to the Window interface, one by one in an ad hoc manner. The NewWindowOptions type already lists things like fullscreen, title, icon, cursorHidden in its TODO comments. There are undoubtedly more we could add. I'd like to have some sort of principled approach to where we draw the line.

One consideration is that not all systems support programmatic resizing. For example, on mobile, the app has no say in its window size. Also, on X11, strictly speaking, the app can only make a request, and it's perfectly valid for the window manager, a separate program, to set the app window's size to be something different, such as what tiling window managers do.

@rsc rsc added this to the Unreleased milestone Dec 28, 2015
@gopherbot
Copy link

@gopherbot gopherbot commented Jan 19, 2016

CL https://golang.org/cl/18740 mentions this issue.

@belak
Copy link

@belak belak commented Jul 6, 2017

What's the status on this? Additionally an option to fullscreen windows would be huge.

@DeedleFake
Copy link

@DeedleFake DeedleFake commented Jul 6, 2017

I'd definitely like to know, too. If it's still a syntax issue, then I'd like to cast a vote for the functional parameters style.

Also, congratulations, @belak. You're the first person I've run into on here with a Quote avatar. No prizes. Sorry.

@nigeltao
Copy link
Contributor

@nigeltao nigeltao commented Jul 6, 2017

No status update since my previous comment.

It's not just a syntax issue. To quote my previous comment: "I'd like to avoid incrementally adding a dozen odd SetThis and SetThat methods to the Window interface, one by one in an ad hoc manner... I'd like to have some sort of principled approach to where we draw the line."

@belak
Copy link

@belak belak commented Jul 6, 2017

It could be a single GetWindowProps and SetWindowProps. This would avoid a ton of GetThis and SetThis methods to the interface and would have a clear way to extend it in the future.

Also, same for me @DeedleFake. Not an extremely common avatar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants