svelte-nodegui icon indicating copy to clipboard operation
svelte-nodegui copied to clipboard

wordWrap and (at least) readOnly properties don't seem to work

Open mperreir opened this issue 3 years ago • 4 comments

When trying this code:

<script lang="ts">
    import { onMount } from "svelte";
    import type { NSVElement, RNWindow } from "@nodegui/svelte-nodegui";
    import { Direction } from "@nodegui/nodegui";
    import fetch from "node-fetch";

    /**
     * The exact type for this is: NSVElement<RNWindow>
     * ... However, the Svelte language tools erroneously expect all bind:this values to extend HTMLElement, so
     * for now, we have to rely on reasserting the type.
     */
    let win;
    let urlWidget;
    let dataWidget;
    let jsonData="";

    async function loadPressed(){
        let dataPromise = await loadData(urlWidget.textContent);
        jsonData = JSON.stringify(dataPromise);
    }

    async function loadData(url){
        try{
            let response = await fetch(url);
            let jsonResponse = await response.json();
            return jsonResponse;
        }
        catch(error){
            console.error(error);
        }
    }

    onMount(() => {
        (window as any).win = win; // Prevent garbage collection, otherwise the window quickly disappears!
        (win as NSVElement<RNWindow>).nativeView.show();

        urlWidget.textContent = "https://reqres.in/api/products/3";

        return () => {
            delete (window as any).win;
        };
    });
</script>

<window
    bind:this={win}
    windowTitle="Seafile Share link DL">
    <view class="vertical">
        <view class="horizontal">
            <text>Share link url:</text>
            <lineEdit id="lineEdit" bind:this={urlWidget}/>
            <button text="Load" on:clicked={loadPressed}/>
        </view>
        <view>
            <text id="dataWidget" wordWrap={true} bind:this={dataWidget}>{jsonData}</text>
            <plainTextEdit readOnly={true} text={jsonData}></plainTextEdit>
        </view>
    </view>
</window>

<style>
    /* 
     * CSS has a few gotchas for now.
     * 1) Some values need to be enclosed with quotes (e.g. `width: '100%';` rather than `width: 100%;`).
     *    See: https://github.com/nodegui/svelte-nodegui/issues/4
     * 2) Classes are not supported yet; they're a bit weird in Qt5.
          See: https://github.com/nodegui/svelte-nodegui/issues/6
     * 3) You can't write element-level rules like `text { color: 'red'; }`, unless they're global (not scoped).
     *    For scoped rules, you have to refer to the underlying native element, e.g. `QLabel { color: 'red'; }`.
     *    See: https://github.com/nodegui/svelte-nodegui/issues/7
     */
    .vertical{
        flex-direction: column;
    }

    .horizontal{
        flex-direction: row;
    }

    #lineEdit{
        flex: 1;
    }
</style>

the text widget does not wrap and the plainTextEdit is not readonly.

mperreir avatar Mar 20 '21 23:03 mperreir

I've found the reason for the issue. Svelte is compiling the attributes from camel-case to lower-case before passing them to Svelte NodeGUI. This is something I'd thought we'd solved once and for all with the PRs to Svelte from @halfnelson and @mrsauravsahu to support foreign namespaces. The Svelte NodeGUI preprocessor (and Svelte Native preprocessor from which it is forked) implicitly inserts <svelte:options namespace="foreign" /> to indicate that our host is foreign and so should not be treated as HTML. But it appears there are still some further cases where it is mistreated.

<window windowTitle="abc"/> correctly preserves casing, i.e. it results in a correct call to widget.setProps({ windowTitle: "abc" }, {}).

However, <text wordWrap={true}>abc</text> strips casing, resulting in an incorrect call to widget.setProps({ wordwrap: true }, {}).

Incidentally, <text wordWrap>abc</text> also strips casing, resulting in an incorrect call to widget.setProps({ wordwrap: "" }, {}).

I also tried <text wordWrap="abc">abc</text>, which also strips casing, resulting in an incorrect call to widget.setProps({ wordwrap: "abc" }, {}).

So it has nothing to do with the values being booleans or strings. I guess Svelte must be special-casing certain attributes from the HTML5 spec, as wordwrap and readonly.

I can't find any mentions of wordwrap in the Svelte repo, but there are some indications that readonly may be treated as a special case in some manner:

  • https://github.com/sveltejs/svelte/blob/c24b313b1af8c55db4fe0ce1ea689000042a2531/test/runtime/samples/attribute-boolean-case-insensitive/main.svelte#L1
  • https://github.com/sveltejs/svelte/blob/c24b313b1af8c55db4fe0ce1ea689000042a2531/src/compiler/compile/render_ssr/handlers/shared/boolean_attributes.ts#L23
  • https://github.com/sveltejs/svelte/blob/a7eff8894fac1254466b885665ada783e82e3cb8/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts#L384

I'm just surprised that this is biting us when we're in a foreign namespace. Does this analysis seem plausible @halfnelson, @mrsauravsahu?

Mentioned PRs:

  • https://github.com/sveltejs/svelte/pull/5652
  • https://github.com/sveltejs/svelte/pull/5942

shirakaba avatar Mar 27 '21 16:03 shirakaba

A horrible workaround for now if it's blocking you:

// BetterText.svelte
<script lang="ts">
    import type { NSVElement, RNText } from "@nodegui/svelte-nodegui";

    let ref: any;
    export let wordwrap: boolean = false;
    // Find some way to map out the other attributes you need.
    // There is probably a clever Svelte way to extend all those of RNTextProps, but I don't know how!
    $: {
        if(ref){
            /** With reference to the setTextProps() method in: src/dom/react-nodegui/src/components/Text/RNText.ts */
            (ref as NSVElement<RNText>).nativeView.setWordWrap(wordwrap);
        }
    }
</script>
<text bind:this={ref}><slot></slot></text>

And use like this:

<script lang="ts">
    import BetterText from BetterText.svelte
</script>

<BetterText wordwrap={true}>abc</BetterText>

shirakaba avatar Mar 27 '21 17:03 shirakaba

Far from perfect, but at least it allows to set wordWrap and readOnly on components that need it :-).

mperreir avatar Mar 27 '21 17:03 mperreir

Looking at the Svelte JS output of the same code in the repl (https://svelte.dev/repl/a64f21f99f3d48de992a731a76818c32?version=3.37.0) gives us a clue

c() {
	window_1 = document.createElementNS("https://svelte.dev/docs#svelte_options", "window");
	view2 = svg_element("view");
	view0 = svg_element("view");
	text0 = svg_element("text");
//snip
        attr(text1, "wordwrap", text1_wordwrap_value = true);

The elements are being detected as and forced to SVG. There must be some code in svelte that does this which we need to skip for namespace foreign

halfnelson avatar Apr 28 '21 20:04 halfnelson