qooxdoo / qooxdoo

qooxdoo - Universal JavaScript Framework
http://qooxdoo.org
Other
764 stars 259 forks source link

Enhance Jsx #10637

Closed WillsterJohnsonAtZenesis closed 3 months ago

WillsterJohnsonAtZenesis commented 6 months ago

The Jsx system now has custom elements, slots, and several other useful pieces of functionality. There is also a setup for <qx:* /> special elements to be added in future for any purposes, which reserves the qx: namespace in the meantime.

Custom Elements

Any function or constructor can be used as a custom element so long as it's name begins with an uppercase letter or it's accessed by property index. It also must return an instance of qx.html.Node so that it can be added to the VDOM.

Slots

Dynamic & declarative way of adding children to predefined locations within a custom element's Jsx structure. This is syntax sugar for the new .inject() method available to elements.

CSS Custom Properties

More lightweight applications may benefit from leaning in to using the VDOM directly in places, creating a small collection of Jsx components. As styling can be more easily done via CSS for Jsx expressions, it makes sense to provide a simple api for passing CSS variables. <MyElem __css_var="red"> will apply --css_var: red; to the cascade from MyElem downwards.

WillsterJohnsonAtZenesis commented 5 months ago

I've just pushed a few changes regarding fixing bugs, passing tests and fleshing out the documentation, but I'll also add some more context here.

the PR says something about existing jsx support - I couldn't find anything in the documentation on this?

@johnspackman added Jsx support in 2021. He tells me he meant to add some docs for it, but must've forgotten for some reason.

could you add some notes on how this ties into a regular qx application

It doesn't. At least not directly.

I briefly mentioned it in the updated SSR docs, and I'll give some more context here. The benefit we've been getting from Jsx is in the generation of printable documents for reports and invoices. Some of these documents are small, maybe only one page, but some of theme are huge - clocking in at hundreds of pages.

These documents are impossible to get right with Qooxdoo desktop, especially when we want to have a pagination process which flows and fits to the physical paper during printing. Aside from the runtime overhead getting out of hand, the development process itself feels excessive - all we need for this is to put data on a page.

In addition to that, getting the styles and layout to work in print is a lot of work for not a particularly brilliant result. Because Qooxdoo desktop is for applications, print media isn't something it has mature APIs for; nobody needs to print an app.

Using Jsx has solved all of these issues and made the development of these reports significantly faster and simpler.

Here's what this Jsx proposal is not doing; it's not a competitor or alternative to Qooxdoo desktop. QxJsx has no reactivity at all, something quite crucial for building an application. It might be possible to mess around with bindings, but with how difficult it is to access deeply nested elements in a Jsx expression, that would be considerably more effort than just using Qooxdoo desktop.

The main purpose here is to fill a gap which qooxdoo desktop leaves open, that being quick, simple, and effective generation of printable and/or server-rendered static documents.

oetiker commented 5 months ago

@WillsterJohnsonAtZenesis that sounds cool ... so lets have some documentation done, in a way that everyone can make use of the new abilities ... examples please :)

hkollmann commented 5 months ago

Seems that you break some tests....

TESTTAPPER: Running tests in chromium

Start watching ... not ok 1277 - qx.test.bom.Basic:testElementAttributes - [31.40 ms] - Expected value to be the CSS color '0,0,0' (rgb(0,0,0)), but found value 'rgba(0, 0, 0, 0)' (rgb(-1,-1,-1))! not ok 1280 - qx.test.bom.Blocker:testBlockElement - [5.80 ms] - Expected '199' but found 'auto'! not ok 1281 - qx.test.bom.Blocker:testBlockerColor - [5.70 ms] - Expected '#ff0000' but found 'rgba(0, 0, 0, 0)'! not ok 1447 - qx.test.bom.element.Style:testSetFloat - [12.40 ms] - Expected 'left' but found ''undefined''! not ok 2887 - qx.test.mobile.core.Widget:testEnabled - [9.50 ms] - Expected 'none' but found 'auto'! not ok 2890 - qx.test.mobile.dialog.Menu:testSetListHeight - [37.00 ms] - Expected '10800' but found '7200'! not ok 2891 - qx.test.mobile.dialog.Menu:testMaxListHeight - [55.80 ms] - Expected '375' but found '792'!

johnspackman commented 5 months ago

My app with this PR is broken. Got the next error:

Looks like this PR is not BC atm.

Could you provide an example?

WillsterJohnsonAtZenesis commented 5 months ago

My app with this PR is broken.

I can't find a way to reproduce this error, would you be able to provide a reproduction/steps to reproduce? That error will only occur if an element is told to use a dom node after it has already acquired one. As far as I'm able to tell the loadNode function shouldn't be attempting to do that. If it is, then that certainly needs fixing.

goldim commented 5 months ago

I am not sure that I can because app is very big. I used to create own custom HTML elements based widgets. Maybe this code could be sent here?

johnspackman commented 5 months ago

I am not sure that I can because app is very big. I used to create own custom HTML elements based widgets. Maybe this code could be sent here?

If you could put a snippet in that would be great - the issue is where custom DOM is then being loaded into the VDOM methods (ie the opposite of the serialize method)

goldim commented 5 months ago

@johnspackman I'll try. Usually I work with SVG and create DOM for it and put it into virtual dom. I think this case.

goldim commented 5 months ago

@WillsterJohnsonAtZenesis Here you are:

qx.Class.define("Widget",
{
    extend : qx.ui.core.Widget,

    members :
    {
        /**
         * Overwritten from qx.ui.core.Widget.
         */
        _createContentElement : function(){
            const el = new qx.html.Element;
            const str = `<svg width="100%" height="100%"><circle cx="50%" cy="50%" r="50%" stroke="black" stroke-width="3" fill="red" /></svg>`;
            el.useMarkup(str);
            return el;
        }
    }
});

var doc = this.getRoot();
doc.add(new Widget(500, 500),
{
  left : 100,
  top  : 50

});
hkollmann commented 3 months ago

What's the state of this MR?

goldim commented 3 months ago

What's the state of this MR?

I wait for author response on the bug which I found.

@WillsterJohnsonAtZenesis could you check my case above please? It doesn't work.

johnspackman commented 3 months ago

What's the state of this MR?

I wait for author response on the bug which I found.

@WillsterJohnsonAtZenesis could you check my case above please? It doesn't work.

We couldnt reproduce it, but i came across it today - i've written a fix but need to do some testing first

goldim commented 3 months ago

@johnspackman I tried and reproduced the error once again. Steps: 1) Download WillsterJohnsonAtZenesis:jsx-enhance 2) npm ci and bootstrap-compiler 3) create some test project npx your\path\to\qooxdoo\bootstrap\qx create test --type=desktop --out=test -I 4) add my code to project (separate class and use it from app class for example) 5) run npx your\path\to\qooxdoo\bootstrap\qx serve -S 6) open page with app and if you don't see anything then u got an error in console

It would be great if somebody of reviewers tries it too to confirm the error. OS on which I tested is Windows 10 and Node v18.16.0.

johnspackman commented 3 months ago

That's because I've written the fix but not added it to the PR yet :).

Literally came across it late yesterday afternoon and got my use case working but want to run the unit tests etc before committing to the PR

WillsterJohnsonAtZenesis commented 3 months ago

@WillsterJohnsonAtZenesis Here you are:

[...]

@goldim I've checked your demo with the new changes this morning and it seems to be working correctly

goldim commented 3 months ago

@WillsterJohnsonAtZenesis Here you are: [...]

@goldim I've checked your demo with the new changes this morning and it seems to be working correctly

Thank you, I will see it today-tomorrow.

goldim commented 3 months ago

@WillsterJohnsonAtZenesis Yes, it fixed my problem. Thank you. I don't use jsx in my qx apps and would like to know if this is a proper way to do it in qx:

qx.Class.define("Widget",
{
    extend : qx.ui.core.Widget,

    members :    {
        _createContentElement : function(){
            return <><h1>aaa</h1><h2>bbb</h2></>;
        }
    }
});

Just I get the next error:

Widget.js:1485 Uncaught TypeError: el.connectObject is not a function
    at wrapper.__createContentElement__P_21_1 (Widget.js:1485:10)
    at wrapper.construct (Widget.js:57:34)
    at wrapper [as constructor] (Class.js:1848:39)
    at wrapper.defaultConstructor (Class.js:1800:33)
    at new wrapper (Class.js:1848:39)
    at wrapper.main (Application.js:51:15)
    at Object.ready (BaseInit.js:77:28)
    at Direct.js:133:37
    at Function._true [as then] (Utils.js:149:22)
    at Direct.js:132:26
    at Array.forEach (<anonymous>)
    at wrapper.dispatchEvent (Direct.js:107:19)
    at wrapper.wrappedFunction [as dispatchEvent] (Interface.js:529:31)
    at wrapper.dispatchEvent (Manager.js:958:22)
    at Registration.js:361:40
    at Function._true [as then] (Utils.js:149:22)

but if it is:

 return <div><h1>aaa</h1><h2>bbb</h2></div>;

everything is fine.

WillsterJohnsonAtZenesis commented 3 months ago

@WillsterJohnsonAtZenesis Yes, it fixed my problem. Thank you. I don't use jsx in my qx apps and would like to know if this is a proper way to do it in qx:

The second option looks more correct to me.

A fragment, ie the tags without a tagname, are just syntax sugar for creating an array. Eg, myJsx and myArray below are equivalent;

const myJsx = (
  <>
    <p>Foo bar</p>
    <p>Hello world</p>
  </>
);

const myArray = [
  <p>Foo bar</p>
  <p>Hello world</p>
];

With this comparison hopefully you can see why the error el.connectObject is not a function appears, as el in this case would be an array.

I think we're actually using qx.data.Array rather than a native Array for fragments, but the effect is the same.

The new 'SSR' and 'Using Jsx' docs pages provide a little more info on using Jsx to compliment a desktop app, though using Jsx directly within a Desktop app by overriding _createContentElement is also valid.