ArthurClemens/Polythene

Modular implementation of Material Design for Mithril.

Cory Mawhorter

@cmawhorter

the problem is that polythene deps use "^x.x.x" instead of a fixed version. for example, i've installed polythene-mithril@1.5.4 specifically, but its still installing polythene-css-* at 1.7.4

i have cyano-mithil in my package-lock file even though at the time 1.5.4 was published that wasn't a thing

flems will never see this problem because its related to npm/node and deps

Arthur Clemens

@ArthurClemens

Yes, the ^ was a mistake. That got fixed in 1.6.5

Cory Mawhorter

@cmawhorter

ok, just reduced it enough for a repro:

create a new empty package.json
npm i polythene-css@1.5.4 polythene-mithril@1.5.4 polythene-utilities@1.5.4
open package-lock and examine what's installed.

"polythene-mithril-base" is 1.5.4 but "polythene-mithril-button" and all the others are 1.7.4

Arthur Clemens

@ArthurClemens

What is specific to version 1.5.4 that you need?

Cory Mawhorter

@cmawhorter

nothing in 1.5.4 specifically. i just upgrade to 1.7.4 and noticed some of my stuff broke and was trying to fix it. i'm seeing the same problem that was mentioned a while back here in chat

and i was trying to revert to last known working and ran into that other problem. i'll try 1.6.5 and see if that resolves it

Cory Mawhorter

@cmawhorter

^== damn. no luck. i'm still seeing issues with scrollElRef being undefined

Arthur Clemens

@ArthurClemens

could you provide some real code?

Cory Mawhorter

@cmawhorter

sure thing -- this seems to be the root cause here:

    m(Tabs, {
      tabs: [
        { _id: 'sections', label: 'Sections' },
        { _id: 'settings', label: 'Theme' },
      ],
      onChange({ options }) {
        vnode.state.selectedSidebarTab = options._id;
        // when changing between tabs, reset tabs to their home state
        clearSettingsBeingEdited(vnode);
      },
    })

if i remove the onChange the problem goes away

i tried wrapping the body of the handler in a setTimeout and that doesn't work. i was just about to add a longer delay

Arthur Clemens

@ArthurClemens

Which mithril version are you using here?

Cory Mawhorter

@cmawhorter

1.1.7. and adding a 1s delay to that handler body firing (with a redraw added) makes the error go away

i guarantee its my code causing the problem. this thing was thrown together

Arthur Clemens

@ArthurClemens

You you try to change https://codesandbox.io/s/polythene-for-mithril-tabs-issue-with-mithril-1x-owvjo to your setup? (as this is working fine)

Cory Mawhorter

@cmawhorter

thanks, i'll take a look at that and see if i can get you any useful info. just in case it helps heres the full ugly function: https://gist.github.com/cmawhorter/0d4a8d73d607d2f93de73588ded6b502

something i neglected to mention is i'm using drawers with a key. not sure if that's part of the issue. i'll give your link a shot and find out

Cory Mawhorter

@cmawhorter

oooook. i think i have a repro. it seems to only happen when the tab body contains a lot of content

i tried adding an onbeforeremove to simulate a lot of teardown but that did nothing. it was only when i created a ton of nodes in a tab body that i could get it to happen

to get it to happen just switch between the tabs back and force rapidly and it'll eventually throw

no idea if drawer is involved

hmmm. though i'm not sure if that's my problem or just a different problem. my link still has the onbeforeremove delay and removing that makes that error go away. dfslkjfdklsjfklerjklwjlka

Cory Mawhorter

@cmawhorter

FWIW i'm giving up and wrapping the handler in a setTimeout and a delay

Arthur Clemens

@ArthurClemens

The tabs do work fine with Mithril 2.x

Cory Mawhorter

@cmawhorter

yeah, but mithril v2 has breaking changes and my app is large and i'm dev and QA. IMO you should unpublish anything > 1.5.4 and republish as v2 since anyone upgrading from 1.5 will experience breaking changes

or remove support for mithril v1

or maybe you could publish v1.5.5 w/deps fixed :heart: ;]

Arthur Clemens

@ArthurClemens

I’ll look into it

Cory Mawhorter

@cmawhorter

i tried to put together a PR but i'm having problems getting the dev env running. thanks for all the help now and in the past Arthur!

Arthur Clemens

@ArthurClemens

@cmawhorter You can now use fixed-version-1.5.6

Carlo Guglielmin

@cgug

Noticing the Polythene tabs reset their selected state on browser resize, so if I have tab 1 selected and resize it goes back to 0. Do I need to do a manual workaround for this or am I missing a param or something?

Arthur Clemens

@ArthurClemens

It’s a bug.

Arthur Clemens

@ArthurClemens

Fix is underway

Arthur Clemens

@ArthurClemens

Released in 1.7.5

Carlo Guglielmin

@cgug

Great, thanks a lot!

Another issue I noticed is in TextField using oninput, if you type a value in, then click to move the cursor to the start of the line, then rapidly type a lot of keys the caret eventually jumps to the end of the input. I figure it's something with oninput timing with the value, as using onchange instead avoids this behaviour

Carlo Guglielmin

@cgug

m(TextField, {
    label: 'Username',
    floatingLabel: true,
    value: username,
    events: {
        oninput: function(e) {
            username = e.target.value;
        }
    }
})

Arthur Clemens

@ArthurClemens

In which browser do you see this happen?

Carlo Guglielmin

@cgug

Chromium 87 and FF 84 on Linux

Arthur Clemens

@ArthurClemens

I don’t run Linux and I also don’t see it offered on BrowserStack. I’ll try some other browsers to see if I can reproduce it.

Carlo Guglielmin

@cgug

Also was reproducable on OS X

I can reboot to Windows and try there if that helps. Seems pretty consistent so I don't think the OS matters

Arthur Clemens

@ArthurClemens

I am using Mac OS with Chromium. I don’t see it happen on any field in the kitchensink: https://arthurclemens.github.io/polythene-demos/mithril/#/textfield

Carlo Guglielmin

@cgug

I tested there as well and couldn't reproduce it earlier. But do those have oninput linked up as well as value? Because if I remove the oninput event or switch it to onchange the problem is fixed

Arthur Clemens

@ArthurClemens

No, some use TextField prop onChange

Carlo Guglielmin

@cgug

Yeah, works fine with onchange, but not oninput

Arthur Clemens

@ArthurClemens

onChange (with capital C) is not the HTML function onchange

https://github.com/ArthurClemens/polythene/blob/master/docs/components/mithril/textfield.md#reading-and-setting-the-value

Where communities thrive

People

Repo info

Activity