#1427 js: fwt.window.root possible error

pervukhin Mon 28 Feb 2011

I want to display a fwt window as a part of the webpage, not attached to document.body but to some other element. And I found that the corresponding code in WindowPeer.js contains an error:

WindowPeer.js, lines 27-37

var shell = document.createElement("div")
with (shell.style)
{
  position   = this.root === document.body ? "fixed" : "absolute";
  top        = "0";
  left       = "0";
  width      = "100%";
  height     = "100%";
  background = "#fff";
}

If this.root is not the document.body, the position is absolute, but it should be relative instead.

I guess it should be fixed, or I'm not using it the right way?

andy Mon 28 Feb 2011

The Window is absolute to the nearest positioned parent, so it requires the parent to be relative. See examples/js/mount.fan for a full example.

pervukhin Wed 2 Mar 2011

Thanks for answer, it works now.

However, other issue raised. If I've attaches to the element other that document.root, the coords within mouse events are incorrect. They shifted down-right by the offset of my root element. I think it is because WidgetPeer.posOnDisplay relies on m_pos, and m_pos is still (0,0) in this case.

Is it a bug?

andy Wed 2 Mar 2011

Promoted to ticket #1427 and assigned to andy

Yeah sounds like a bug - will take a look.

Yuri Strot Sat 5 Mar 2011

This should work both for Dialog and Window:

diff -r 2727c1c69346 src/fwt/js/WidgetPeer.js
--- a/src/fwt/js/WidgetPeer.js	Wed Feb 23 15:30:05 2011 -0500
+++ b/src/fwt/js/WidgetPeer.js	Sat Mar 05 19:28:00 2011 +0600
@@ -49,11 +49,14 @@
     if (p instanceof fan.fwt.Tab) p = p.parent();
     x += p.peer.m_pos.m_x - p.peer.elem.scrollLeft;
     y += p.peer.m_pos.m_y - p.peer.elem.scrollTop;
-    if (p instanceof fan.fwt.Dialog)
+    if (p instanceof fan.fwt.Window)
     {
-      var dlg = p.peer.elem.parentNode;
-      x += dlg.offsetLeft;
-      y += dlg.offsetTop;
+      var elem = p.peer.elem;
+      if (elem.offsetParent)
+      {
+        do { x += elem.offsetLeft; y += elem.offsetTop; }
+        while (elem = elem.offsetParent);
+      }
     }
     p = p.parent();
   }

andy Thu 14 Apr 2011

Ticket resolved in 1.0.59

Tracked this down to where I was mapping the DOM event position to FWT widget. So for now I am going to keep the posOnDisplay relative to root Window to keep things simple.

Yuri Strot Thu 21 Apr 2011

If you want to keep posOnDisplay as is, you need to fix fan.fwt.MenuPeer.prototype.relayout as well. Now we have an issue with fwt::Menu appearance which doesn't take into account root element shift. Also fan.fwt.TablePeer.prototype.$onMouseDown using posOnDisplay and should be verified.

Actually I think it's more powerful to make screen for JavaScript whole browser window. It's more closer to what we have in desktop and fandoc for fwt::Widget.pos, fwt::Widget.bounds, etc. already mentioned that window has coordinates relative to screen.

For simplicity we can provide both:

Point? posOnDisplay()
Point? posOnWindow()

andy Thu 21 Apr 2011

Thanks Yuri - pushed a fix for Menu.

I like posOnWindow if you want to work up a patch?

Yuri Strot Thu 21 Apr 2011

Thanks for quick fix Andy!

Are you talking about moving to document coordinates + posOnWindow method? Yeah, I can work on this.

andy Thu 21 Apr 2011

Are you talking about moving to document coordinates + posOnWindow method?

We need 3 things I think:

  1. Add Widget.posOnWindow for SWT
  2. Rename current posOnDisplay for JS to posOnWindow (and all uses)
  3. Then add a new posOnDisplay for JS for actual document position

If you can send those as separate changesets too, that would really help, since I need to verify alot of code at each step.

Yuri Strot Thu 21 Apr 2011

Ok, it makes sense. I'll work on this right after ScrollBar improvements.

Yuri Strot Fri 22 Apr 2011

I implemented the first two parts. It's not too much code:

diff -r c400a4cc4f43 src/fwt/fan/Widget.fan
--- a/src/fwt/fan/Widget.fan	Thu Apr 21 11:56:05 2011 -0400
+++ b/src/fwt/fan/Widget.fan	Fri Apr 22 17:30:24 2011 +0700
@@ -249,6 +249,12 @@
   }
 
   **
+  ** Get the position of this widget relative to the window.
+  ** If not on mounted on the screen then return null.
+  **
+  native Point? posOnWindow()
+
+  **
   ** Get the position of this widget on the screen coordinate's
   ** system.  If not on mounted on the screen then return null.
   **
diff -r c400a4cc4f43 src/fwt/java/WidgetPeer.java
--- a/src/fwt/java/WidgetPeer.java	Thu Apr 21 11:56:05 2011 -0400
+++ b/src/fwt/java/WidgetPeer.java	Fri Apr 22 17:30:24 2011 +0700
@@ -138,6 +138,17 @@
     return size(s);
   }
 
+  public fan.gfx.Point posOnWindow(fan.fwt.Widget self)
+  {
+    if (!(control instanceof Control)) return null;
+    fan.fwt.Window window = self.window();
+    if (window == null || !(window.peer.control instanceof Control)) return null;
+    Control widgetControl = (Control)control;
+    Control windowControl = (Control)window.peer.control;
+    Point pt = Fwt.get().display.map(widgetControl, windowControl, 0, 0);
+    return point(pt);
+  }
+
   public fan.gfx.Point posOnDisplay(fan.fwt.Widget self)
   {
     if (!(control instanceof Control)) return null;
diff -r c400a4cc4f43 src/fwt/js/MenuPeer.js
--- a/src/fwt/js/MenuPeer.js	Thu Apr 21 11:56:05 2011 -0400
+++ b/src/fwt/js/MenuPeer.js	Fri Apr 22 17:30:24 2011 +0700
@@ -103,7 +103,7 @@
     ph += mh;
   }
 
-  var pp = this.$parent.posOnDisplay();
+  var pp = this.$parent.posOnWindow();
   var ps = this.$parent.size();
   var x = pp.m_x + this.$point.m_x;
   var y = pp.m_y + this.$point.m_y;
diff -r c400a4cc4f43 src/fwt/js/WidgetPeer.js
--- a/src/fwt/js/WidgetPeer.js	Thu Apr 21 11:56:05 2011 -0400
+++ b/src/fwt/js/WidgetPeer.js	Fri Apr 22 17:30:24 2011 +0700
@@ -39,7 +39,7 @@
   return self;
 }
 
-fan.fwt.WidgetPeer.prototype.posOnDisplay = function(self)
+fan.fwt.WidgetPeer.prototype.posOnWindow = function(self)
 {
   var x = this.m_pos.m_x;
   var y = this.m_pos.m_y;
@@ -61,6 +61,12 @@
   return fan.gfx.Point.make(x, y);
 }
 
+fan.fwt.WidgetPeer.prototype.posOnDisplay = function(self)
+{
+  //equals to posOnWindow for now
+  return this.posOnWindow(self);
+}
+
 fan.fwt.WidgetPeer.prototype.prefSize = function(self, hints)
 {
   // cache size
@@ -187,7 +193,7 @@
     var func = function(e)
     {
       // find pos relative to widget
-      var dis  = peer.posOnDisplay(self);
+      var dis  = peer.posOnWindow(self);
       var mx   = e.clientX - dis.m_x;
       var my   = e.clientY - dis.m_y;
 

andy Fri 22 Apr 2011

That looks good - pushed your patch.

Yuri Strot Mon 25 Apr 2011

To finish this I think we need to clean up fwt structure:

  1. Each attached widget has root window.
  2. Each window appears on the screen and has a position relative to this screen
  3. We can move window over screen.
  4. For desktop screen is a monitor by default. For browser screen is a browser window by default.

This looks good. Now we want to use fantom widgets as part of existing native structure. For example we can use fantom written chart in a web site or in a eclipse view. In JavaScript we have fwt.window.root variable. In SWT we currently using a lot of ugly reflection. So I want to find a better way to get the job done.

The are list of questions which appear when we attach our Widget to some native component (element in DOM or Composite in SWT):

  • Can we attach Widget to native component without Window?
  • What is screen in this case?
  • How we should attach fantom widget to native component? fwt.window.root variable doesn't look universal approach because in SWT we haven't unique id for composite. Also how we can attach several widgets to different components?

Did you guys think about it?

andy Mon 25 Apr 2011

Not sure I quite followed your question/proposal?

In the web, you can already mount your Window anywhere in the DOM.

But I could see value in mounting an FWT widget tree into Eclipse - could see something like an "EclipsePane" that does the glue.

Yuri Strot Mon 25 Apr 2011

Let me show you example to clarify:

window := Window { pos = Point(100, 100); content = ... }
window.open
...
echo(window.posOnDisplay) //~= 100,100 for SWT; 0,0 fow JS
window.pos = Point(200, 200)
echo(window.posOnDisplay) //~= 200,200 for SWT; 0,0 for JS

In fact we can implement the same for JavaScript as part of moving to screen-based coordinates.

Now consider we mounting window to DOM element. How my example should work? Can we change position? And where is "display" in this case? There are my questions.

andy Mon 25 Apr 2011

FWT for the browser is implemented such that fwt::Window is more-or-less a proxy to the browser window - it doesn't treat the viewport as a Desktop space. So it works a bit differently from SWT when you down to that level.

Once posOnDisplay is implemented properly - it should return the offset of the Window node relative to the document origin.

Yuri Strot Wed 27 Apr 2011

I'm agree on this. Now I want to show you another use case of FWT: make some components with FWT and integrate in to native application. Let's say I created ChartWidget. How I can put it into my website? How I can use several charts on the one page?

As you mentioned Window in JavaScript is not intended to have separate instances while it's only a proxy to the browser window. So it's not a good solution to use fwt.window.root variable and open several windows attached to different DOM elements. Moreover it's not only JavaScript issue because I want to do the same in my desktop application.

I think in case of mounting we shouldn't open a Window. The same for SWT - if you want to add some FWT widget to your native application you shouldn't open a new Window. So the right approach for me is to mount a widget directly to native elements/controls without needless Window. In this case chart.posOnDisplay should work as you proposed and chart.window should return null. Does it make sense?

andy Wed 27 Apr 2011

My gut says we would create a lot of problems for ourselves if we didn't assume we were mounted in a fwt::Window. That's not to say you can't mount into SWT - but you need some layer there to model the SWT parent tree as a valid FWT tree - all the way up to Window - so everything works as expected.

The only big issue with not supporting multiple Window instances in the browser - is simply having a good solution for mapping window instances to DOM nodes. If there was an elegant solution there that supported multiple windows, I would be open to supporting that.

Also keep in mind there is no reason you have to use FWT here - you could just run Fantom and work with gfx and dom pods to do things like charts at a lower level - and indeed our chart package for SkySpark is essentially stand-alone - just mounts in a FWT widget wrapper.

Yuri Strot Fri 8 Jul 2011

The last patch for posOnDisplay support:

diff -r 7401f5f23327 src/fwt/js/MenuPeer.js
--- a/src/fwt/js/MenuPeer.js	Fri Jun 17 15:42:35 2011 -0400
+++ b/src/fwt/js/MenuPeer.js	Fri Jul 08 15:36:30 2011 +0700
@@ -103,21 +103,13 @@
     ph += mh;
   }
 
-  var pp = this.$parent.posOnWindow();
+  var pp = this.$parent.posOnDisplay();
   var ps = this.$parent.size();
   var x = pp.m_x + this.$point.m_x;
   var y = pp.m_y + this.$point.m_y;
   var w = pw;
   var h = ph;
 
-  // adjust for window root
-  var win = this.$parent.window();
-  if (win != null && win.peer.root != null)
-  {
-    x += win.peer.root.offsetLeft;
-    y += win.peer.root.offsetTop;
-  }
-
   // check if we need to swap dir
   var shell = this.elem.parentNode;
   if (x+w >= shell.offsetWidth-4)  x = pp.m_x + ps.m_w - w -1;
diff -r 7401f5f23327 src/fwt/js/WidgetPeer.js
--- a/src/fwt/js/WidgetPeer.js	Fri Jun 17 15:42:35 2011 -0400
+++ b/src/fwt/js/WidgetPeer.js	Fri Jul 08 15:36:30 2011 +0700
@@ -63,8 +63,24 @@
 
 fan.fwt.WidgetPeer.prototype.posOnDisplay = function(self)
 {
-  //equals to posOnWindow for now
-  return this.posOnWindow(self);
+  // find pos relative to window
+  var pos = this.posOnWindow(self);
+  var win = self.window();
+  if (win != null && win.peer.root != null)
+  {
+    // find position of window relative to display
+    var elem = win.peer.root;
+    var x = 0, y = 0;
+    do
+    {
+      x += elem.offsetLeft - elem.scrollLeft;
+      y += elem.offsetTop - elem.scrollTop;
+    }
+    while(elem = elem.offsetParent);
+    if (x != 0 || y != 0)
+      return fan.gfx.Point.make(pos.m_x + x, pos.m_y + y);
+  }
+  return pos;
 }
 
 fan.fwt.WidgetPeer.prototype.prefSize = function(self, hints)
@@ -276,19 +292,11 @@
   var peer = this;
   var func = function(e)
   {
-    // find pos relative to widget
-    var dis  = peer.posOnWindow(self);
+    // find pos relative to display
+    var dis  = peer.posOnDisplay(self);
     var mx   = e.clientX - dis.m_x;
     var my   = e.clientY - dis.m_y;
 
-    // make sure to rel against window root
-    var win = self.window();
-    if (win != null && win.peer.root != null)
-    {
-      mx -= win.peer.root.offsetLeft;
-      my -= win.peer.root.offsetTop;
-    }
-
     // cache event type
     var isClickEvent = evtId == fan.fwt.EventId.m_mouseDown ||
                        evtId == fan.fwt.EventId.m_mouseUp;

andy Fri 8 Jul 2011

Yuri - can you clarify what this is for/does?

Yuri Strot Fri 8 Jul 2011

Yeah, sure. We started discussion about fwt::Widget.posOnDisplay which actually returns position relative to a window. Therefore when FWT window is not a root element on the page or if we have some scrolling there was an issue with mouse position calculation. So we made a dicision to start following migration:

  • Add Widget.posOnWindow
  • Rename current posOnDisplay for JS to posOnWindow (and all uses)
  • Then add a new posOnDisplay for JS for actual document position

The first two cases was implemented and now we have both posOnWindow and posOnDisplay which works fine in SWT, but in JavaScript posOnDisplay returns the same value as posOnWindow. Also we're using following trick in JavaScript to handle non-root window case:

var dis  = peer.posOnWindow(self);
var mx   = e.clientX - dis.m_x;
var my   = e.clientY - dis.m_y;

// make sure to rel against window root
var win = self.window();
if (win != null && win.peer.root != null)
{
  mx -= win.peer.root.offsetLeft;
  my -= win.peer.root.offsetTop;
}

The patch I've attached has following:

  • Calculate posOnDisplay as posOnWindow + window position on display.
  • Replace posOnWindow with posOnDisplay where we need to convert mouse coordinates to coordinates relative to Widget (was used for listeners and menus).
  • Remove non-root window workaround.

andy Fri 8 Jul 2011

Ah yeah - I remember - thanks Yuri.

andy Mon 11 Jul 2011

Pushed - thanks Yuri.

Login or Signup to reply.