flarum / issue-archive

0 stars 0 forks source link

Helper method for injecting content into Mithril vdom #228

Open clarkwinkelmann opened 4 years ago

clarkwinkelmann commented 4 years ago

Feature Request

Is your feature request related to a problem? Please describe. When no ItemList is available, extending Mithril vdom is a bit delicate, and not very readable.

Example by @datitisev , add new nodes around the IP address in the post meta dropdown https://github.com/FriendsOfFlarum/geoip/blob/c200c3edffb9cb935093a9c3028a66a4c405144f/js/src/forum/index.js#L52-L53

    const dropdownMenu = vdom.children.find(e => e.attrs && e.attrs.className && e.attrs.className.includes('dropdown-menu'));
    const el = dropdownMenu.children.find(e => e.tag === 'span' && e.attrs && e.attrs.className === 'PostMeta-ip');

    const { description, threat, image } = getIPData(ipInfo);

    el.children[0] = (
        <span /* stuff */>
            {el.children[0]}
        </span>
    );

An example of mine from the Wordpress premium extension, adding disabled to the password field in the user edit modal:

        if (!this.kilowhatWordpressShowFields && items.has('password')) {
            const passwordField = items.get('password');

            if (
                Array.isArray(passwordField.children) &&
                passwordField.children.length > 1 && // .Form-group children
                passwordField.children[1] &&
                Array.isArray(passwordField.children[1].children) &&
                passwordField.children[1].children.length > 1 && // div children
                passwordField.children[1].children[1] &&
                Array.isArray(passwordField.children[1].children[1].children) &&
                passwordField.children[1].children[1].children.length > 0 && // label children
                passwordField.children[1].children[1].children[0] &&
                passwordField.children[1].children[1].children[0].tag === 'input'
            ) {
                if (!passwordField.children[1].children[1].children[0].attrs) {
                    passwordField.children[1].children[1].children[0].attrs = {};
                }
                passwordField.children[1].children[1].children[0].attrs.disabled = true;
            }
        }

I will try to add more example when I find some of the extensions I had to use those dirty tricks.

Describe the solution you'd like

Given we are often looking for specific tags or class names, it would be nice to build a CSS-query based helper method to retrieve child notes.

Maybe something like otherVdom = vdomQuery(vdom, '.Form-group div label input') could return the corresponding node or null.

The selector doesn't have to be CSS-compliant. If it simply looks for the first child that matches the next query part it should work.

There could be situations where what's returned from a method is actually a list of vnodes, so we would have to decide whether the method accepts children directly instead of a vnode.

Also important, contrary to a DOM querySelector() call, we might want to also match the first node that we pass (the "root") as we might want to validate its type as well.

My suggestion doesn't removes all the use of dirty tricks, like preparing .attrs if it's not defined yet, or changing children array when you want to insert before or around it.

The behavior still requires discussion I think. It would probably be a good idea someone implement such a method in an extension that needs it, and we could then copy it over to core when we're certain it's useful.

Justify why this feature belongs in Flarum's core, rather than in a third-party extension Just like other helper methods, it will be available for all extensions to use. It likely won't be used by core itself though.

Describe alternatives you've considered We could aggressively add new ItemLists. This is the preferable option in many situations but there will always be edge cases where the helper method would be useful.

nxta commented 4 years ago

@clarkwinkelmann I saw how you traverse the vdom in flarum-ext-circle-groups and last night I wrote this helper function based off that. I've been applying it in my code today and it turns out to be very handy.

https://gist.github.com/nxta/af8fbcd28f4cd1f2884fe67ffd3f25d1

Take it, change it, use it however you please. It would be cool if it made it into Flarum

nxta commented 4 years ago

Oh and I use this instead of what you described as dirty tricks. Together they clean up a bunch of nasty code.

https://gist.github.com/nxta/f51bc389734d707cc8918a19ea079c7a

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum. In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

dsevillamartin commented 4 years ago

This issue is still relevant.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum. In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

dsevillamartin commented 4 years ago

Still relevant.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We do this to keep the amount of open issues to a manageable minimum. In any case, thanks for taking an interest in this software and contributing by opening the issue in the first place!

iPurpl3x commented 3 years ago

@askvortsov1 here is my implementation of some very handy util functions to handle vdom objects.

askvortsov1 commented 3 years ago

Thank you for sharing! Comparing the two just on the code, I think I prefer https://gist.github.com/nxta/af8fbcd28f4cd1f2884fe67ffd3f25d1 for actually querying vdom as it's a bit more versatile (and doesn't depend on lodash), but I like the idea of a childIndex helper. How often does getChildren end up being used in your applications? And would you say the most common use case of these is to query elements, or to add in new ones?

dsevillamartin commented 3 years ago

I'd personally rather use lodash.result or lodash.get rather than repeat the same snippet 30 times trying to check if all the properties exist. 🤷

askvortsov1 commented 3 years ago

My thought was that it support queries beyond just class name

davwheat commented 2 years ago

I made this util as part of a client project under Blomstra: https://gist.github.com/davwheat/5a375bdb01682c6e1be50f54d5bd2ce2

It supports CSS selectors (id, classes, attributes), but uses a callback syntax for modification so as to better handle when the item is not found in the VDOM.

The main downsides are...