Active-CSS / active-css

The epic event-driven browser language for UI with functionality in one-liner CSS. Over 100 incredible CSS commands for DOM manipulation, ajax, reactive variables, single-page application routing, and lots more. Could CSS be the JavaScript framework of the future?
https://activecss.org
Other
42 stars 7 forks source link

observe event not always working when declared inside components - proposal for @observe for syntax clarity #239

Closed bob2517 closed 2 years ago

bob2517 commented 2 years ago

Workaround until fixed is to move the observe outside of the component. That workaround will work for document scoped components only.

Investigating...

bob2517 commented 2 years ago

Bug repeated in a document-scoped component. Seems ok in a private event and strictlyPrivate event-scoped components.

bob2517 commented 2 years ago

The error I'm getting in my debugging area is not related to the observe event though - it's to do with running custom events with a labelled timer. I'll fix that and see if the observe event still breaks.

bob2517 commented 2 years ago

Fixed a minor memory leak as part of this - "after" delay events weren't cleaning up immediately after the action was performed. They were only getting removed when the element was removed from the DOM.

bob2517 commented 2 years ago

My disrelated error has now been fixed offline.

bob2517 commented 2 years ago

I'm suspecting that this error is to do with nested components, which is why I'm having trouble repeating it. I'll set up a deeply nested component tree and try to repeat the error with that scenario.

bob2517 commented 2 years ago

Observe works in non-custom element embedded nested components in the document scope. Will try deeply-nested custom element components next.

bob2517 commented 2 years ago

I can repeat the problem now.

The problem isn't the observe event - I get the error on the click event too. It works outside the component, but not inside.

The offending syntax is:

    body:if-exists(main-panel list-body list-item):click {
        add-class: .main;
        remove-class: .intro;
    }

Investigating "if-exists" - could be a problem isolated to that.

bob2517 commented 2 years ago

In an component, if the event selector is outside of the component for an event, the event isn't triggering. This is why it hasn't come up as an issue before. It should be triggering if the component itself is in the event scope though. In this case the scope is the document scope. Target selectors are working in the correct scope though.

bob2517 commented 2 years ago

Problem looks to be with the bubbling - it's not getting up to the body.

bob2517 commented 2 years ago

Hmmm... this is an interesting one. The issue has been there since day one. Do we allow elements outside components to have events inside components if they are in the same scope. That is the question. I think the fact that it's intuitively supposed to work that way means that the answer is yes. Bit annoying though - I've not got the performant solution yet, architecturally speaking. I can see in the code what is happening, but I'm hesitant to put heavy checks into the event flow, and I can't see a way around that yet. I'll do some ironing and see if that helps me think up something.

I think this is only an issue for components in the document scope - I'm pretty sure it works for the component logic, but that has it's own separate bubbling thing going on.

bob2517 commented 2 years ago

No sooner had I picked up the iron then I got the solution. Grab the target component from the event and make sure that is passed into the event flow. That way the event flow can still check the original component events when it reaches beyond the component boundaries if the target came from inside. Shouldn't have any loss in performance. The bubbling just needs to break out at the right point to make sure that the selector check is not going outside the event scope. So when this is done, test a body event inside a privateEvent component because that shouldn't work.

dragontheory commented 2 years ago

Ironing always helps clear the mind... lol

bob2517 commented 2 years ago

Yes :) Weird but true.

bob2517 commented 2 years ago

Got it working with the regular event flow offline, so it should be possible to have outer elements have their events in inner document-scoped components.

Not working yet though for the observe event. There's no bubbling in the observe event, so it's a different kettle of piranha altogether and requires a different tactic, for which I may need to embark on further ironing

bob2517 commented 2 years ago

Looking at this freshly today, it may not be necessary to do the observe event, because all these three things can do the same thing, Any element can basically be a host for the observe event, thanks to the cross-DOM nature of the conditionals:

list-item div:if-exists(main-panel list-body list-item):observe {
    body {      
        add-class: .main;
        remove-class: .intro;
    }
}   
body:if-exists(main-panel list-body list-item):observe {
    body {      
        add-class: .main;
        remove-class: .intro;
    }
}   
main-panel div:if-exists(main-panel list-body list-item):observe {
    body {      
        add-class: .main;
        remove-class: .intro;
    }
}   

The correct observe event out of those three, to put into the component itself, is an element from the component itself.

Simple! For my test with 2 nested components, I picked the top one, because the final div under list-item is inside my component, so that is where is can be observed.


I'm just reviewing the changes that I've made to allow document scope outer elements to have events inside components to make sure that bubbling rules are still followed sensibly before I commit this issue.

bob2517 commented 2 years ago

Any element can basically be a host for the observe event, thanks to the cross-DOM nature of the conditionals

It makes me wonder whether or not a more appropriate syntax should be something like:

@observe if-exists(main-panel list-body list-item) {
    body {      
        add-class: .main;
        remove-class: .intro;
    }
}
bob2517 commented 2 years ago

I need to at least try to stop giving myself more work...

bob2517 commented 2 years ago

I think @observe might be heavier on performance though. Need to check it out.

bob2517 commented 2 years ago

You could do this though, if I set it up:

@observe if-exists(main-panel list-body list-item) && myValue == "ready to rock" {
    body {      
        add-class: .main;
        remove-class: .intro;
    }
}

It's like an if statement that sits around waiting for something to happen.

bob2517 commented 2 years ago

I'm tempted to do it regardless, tbh.

bob2517 commented 2 years ago

The only thing that that can't do though... if actions need to happen inside a private component or a shadow DOM, it's not going to work, unless it can itself be nested inside the component, like:

@component myShadow shadow {
    @observe if-exists(main-panel list-body list-item) && myValue == "ready to rock" {
        an-element-inside {     
            add-class: .main;
            remove-class: .intro;
        }
    }
}
bob2517 commented 2 years ago

It does lead towards stability in use - it's easier to understand the rules with @observe than with the observe event. It would even be simpler for me to code in the core (but that depends on my performance concerns which I need to check out).

It would be an advanced use of ACSS though, because I wouldn't say that it's particularly easy to read.

bob2517 commented 2 years ago

I think that could take the event-driven method to the next level though - checking both the state of the DOM and anything else that can be set up in an @if statement, in just one @observe event statement.

Need some feedback. DT?

bob2517 commented 2 years ago

Another name for it could be @dynamic-if. But that maybe reads a bit weird, and it probably will make it look a bit more scary to use. So @observe seems to be the winner in terms of names for the moment.

bob2517 commented 2 years ago

It would only work off the back of DOM mutations though, unless I tied it into other things, but then the performance could degrade quite dramatically. There's no point completely overriding the purpose of all the other events if it's going to slow things up.

So maybe it's more appropriate to call it @mutationObserve? Or is that too cheeky?

bob2517 commented 2 years ago

Nah - I think that's probably going to confuse things. Mutation observe functionality in JS has a completely different functionality setup.

Maybe @domObserve? That feels a bit more family-friendly.

bob2517 commented 2 years ago

Or just @observe?

bob2517 commented 2 years ago

Ok - going to come back to this in a bit.

bob2517 commented 2 years ago

Maybe something for 2.11.0. Need to get this release out as it fixes a few key things.

bob2517 commented 2 years ago

For performance reasons and concerns over bubbling rules and potential confusions for developers there, I'm removing the inner component ability to access outer-level elements that I've done offline. So from henceforth, events attached to specific elements can only be declared in their appropriate areas. So body events cannot be inside components.

This hasn't been an issue in practice. Only the observe event came up recently as something that was indicated. But that can be fixed by changing the host of the observe event to something inside the component.

Unless I get a specific use-case for allowing this which really doesn't make sense unless done from inside the component, I'm going to pend this for now - it was a side-effect of this issue anyway so it's no code will be harmed in the setting of this rule. That was the unwritten rule up to now anyway.

Note to self - this is the code from the main event loop that is enough to reconstruct if it's needed later:

if (compDetails.topEvDoc && compDetails.topEvDoc.nodeType == 9) {   // 9 == a document node type.
    // The target is bubbling up from a document scoped component. Elements above it can be set within the component.
    // Keep the component the same as the target. This will allow the event flow to keep checking the inner events, as well as the outer events.
    // This is a special case for document scoped components, and set up to fit the way the event flow currently works.
    docScopedCompDetails = compDetails;
    // Nullify the variable scoping though - we don't want that bubbled up too - we just want the event scope and the component reference.
    docScopedCompDetails.varScope = false;
    docScopedCompDetails.strictVars = false;
}

if (!compDetails.topEvDoc && docScopedCompDetails) {
    // We've bubbled up from a component in the document scope to the document. Set the original component as the area for this element.
    // This allows the event flow on upper level elements to still call all the component events logic, which is split for speed, as well as the
    // global scope events.
    docScopedCompDetails.push(compDetails);
}

Then in the event flow, add compDetails to the docScopedCompDetails array and loop. It does affect performance, adding more to the overall flow, so only do this if absolutely required.

dragontheory commented 2 years ago

Kettle corn! Yum!

There's no bubbling in the observe event, so it's a different kettle of piranha...

https://www.hy-vee.com/aisles-online/p/1607897/Peaceful-Piranha-Kettle-Corn

I need to at least try to stop giving myself more work...

lol! I have the same problem...

It's like an if statement that sits around waiting for something to happen.

Very interesting...

It would be an advanced use of ACSS though, because I wouldn't say that it's particularly easy to read.

Instead of the observe ALWAYS being at the END of the statement, it could ALSO be at the beginning?

From a seriously biased front-end CSS perspective, this makes more sense to me. Not sure why but seeing the observe at the FRONT of the statement helps me to understand what is going on for the rest of the statement. Like having punctuation at the beginning of a sentence... Also CSS has a lot going on at the end of statements, like pseudo classes and elements that could be confusing to some in how or if they relate to each other.

@observe if-exists(main-panel list-body list-item) && myValue == "ready to rock" {
  body {        
    add-class: .main;
    remove-class: .intro;
  }
}
bob2517 commented 2 years ago

Kettle corn! Yum!

"Whether you're looking to annihilate a bag of crunchy, munchy morsels or decimate some devastatingly sweet delectables, we've got your snack needs covered."

Not sure why but seeing the observe at the FRONT of the statement helps me to understand what is going on for the rest of the statement.

Yes - I've been realising recently that the same thing written in a slightly different syntax can have that effect. I'm convinced there is a skill in designing understandable syntax for programming languages, but I've not seen any articles on it. It's probably a bit too niche.

dragontheory commented 2 years ago

So from henceforth, events attached to specific elements can only be declared in their appropriate areas. So body events cannot be inside components.

This rule sounds like the CSS cascade...

Forgive me for not fully understanding, but is this similar to scope?

Will this rule disallow using @observe to chain multiple if conditional statements together outside of their scope?

For example... If two dissasociated <list-item>s exist in two different scopes is true, add classes in a third and forth scope outside of the first two?

dragontheory commented 2 years ago

Can ACSS be utilized for security and role validation?

For example, if user xyz has role = admin, then add .admin class to <body> that shows the admin panel?

bob2517 commented 2 years ago

Can ACSS be utilized for security and role validation? For example, if user xyz has role = admin, then add .admin class to that shows the admin panel?

To the extent that it can add and remove classes, and do things according to how variables are set, yes. But only as much as it is ok to do it in JavaScript.

You have to watch the security aspects. Like if a hacker could replicate successful authentication just by manipulating the DOM, then they could become logged in. So never rely on authentication that is purely browser-based. You need to have some sort of server to validate security as well, or some key-based security on the front-end that validates with an API or something. But again, you have to watch that this cannot be bypassed by a hacker. It's healthy to be as paranoid as possible when it comes to web security.

bob2517 commented 2 years ago

So from henceforth, events attached to specific elements can only be declared in their appropriate areas. So body events cannot be inside components.

This rule sounds like the CSS cascade...

Forgive me for not fully understanding, but is this similar to scope?

Will this rule disallow using @observe to chain multiple if conditional statements together outside of their scope?

For example... If two dissasociated <list-item>s exist in two different scopes is true, add classes in a third and forth scope outside of the first two?

This is just for writing event selectors. There is no limitation as to what selectors you write. But they need to be written in the right place. Like put an event that runs on the body, like body:click, outside of any component. Write the event that runs on an element inside a component, inside the component, and not in a random place.

Target selectors can be written to target anything (unless you have set up a strictlyPrivate component, and it also gets weird with shadow DOMs, but there is a way to traverse those in ACSS).

@observe event conditionals won't be affected either. You will be able to chain them to go anywhere in the DOM.

This is literally just for event selectors - events that are tied to a specific element must be written in right place. It should make things a bit simpler to organise too.

bob2517 commented 2 years ago

You haven't run into any issues with this yet. This rule has actually been there all along and it hasn't actually stopped you from doing anything. I just mainly need to mention it in the docs, because it's an unwritten rule.

I haven't changed anything in the core to reflect the rule. That's just the way it's always been set up to run.

I won't be making any new changes to the core for this rule, as it has technically existed since the first version of the core. It was intentionally designed that way at the very beginning. I just hadn't particularly mentioned it in the docs.

bob2517 commented 2 years ago

Basically, all you need to bear in mind, is to make sure you write your events in the most appropiate place.

In other words, the rule is:

If you are attaching an event to an element, the event should be written in the code where the element can be found.

That's probably the better way to describe it.

bob2517 commented 2 years ago

Or...

You cannot place an event on an element that is "outside" where you are. If you are inside a component, you cannot write an event for an element outside the component, higher up the DOM.

You can only write events for an element inside an area that you are coding in, and that includes component nesting below it.

You cannot write events for an element for a component above you, if you are inside a component.

You can target the whole DOM from inside any component that does not have any special scoping set up. And you do not currently have any special scoping set up, so it's fine.

bob2517 commented 2 years ago

Let me know if that's not clear. I might have to do a diagram or something.

dragontheory commented 2 years ago

Let me know if that's not clear. I might have to do a diagram or something.

I think I got it and pretty sure others will get it easily enough. That said, as a visual learner, there's nothing like a good diagram for those "AHA!" moments. lol

dragontheory commented 2 years ago

Riddle me this...

In my example, I have a panel-splitter.acss component...

https://github.com/dragontheory/D7460N-EMS/tree/migration/assets/activecss/components/panel-splitter

@component panelSplitter html("/assets/activecss/components/panel-splitter/panel-splitter.html" get nocache) {
  &:componentOpen {
    run: {=
      Split({
        columnGutters: [{
          track: 3,
          element: document.querySelector('panel-splitter'),
          }]
       });
    =};
  }
}

The splitter uses modern CSS grid values to do the moving, which is very desirable to CSS peeps. On mouse down, the panel-splitter grabs the default static CSS grid layout column values in page-container.css...

https://github.com/nathancahill/split/tree/master/packages/split-grid

By default, there are six columns. Two of which are width: 0; to hide them.

(Can't display: none; them because they won't be counted as part of the CSS grid. Plus I want to be able to animate them later. Can't animate display: none; without a huge amount of extra work-around code. Bleh...)

Anyway, when <panel-aside2> is visible, the CSS grid column values are no longer equal to the default. So if the splitter is clicked when the <panel-aside2> is visible, the splitter grabs the default CSS grid values, not the current CSS grid values with the visible <panel-aside2> and thus <panel-aside2> disappears.

https://github.com/dragontheory/D7460N-EMS/blob/migration/assets/activecss/components/page-container/page-container.css

page-container {
  display: grid;
  gap: 1rem;
  padding: 1rem;
  height: 100vh;
  overflow: hidden;
  grid-template-rows: auto 1fr auto;
  grid-template-columns: 250px 0 1fr 5px 300px 0; /* Six columns by default. Two of which are width zero to hide them. */
}

So, how do I get splitter to grab the current CSS grid column values instead of the default CSS grid column values from inside the ACSS file?

Hope that makes sense...

Alternatively, I could have all the CSS grid column values as auto but then I couldn't control the width of each individual column.

Alternative 2, you mentioned that ACSS also does the resizer thing like this. If I remember correctly, it uses the two sided width method rather than the CSS grid column width method. I'm already vested in ACSS. I would love to cut my dependencies in half by using ACSS for resizing if there was an option to use CSS grid column width method rather than two sided width method. WDYT?

bob2517 commented 2 years ago

Do you know how to do it in JavaScript?

bob2517 commented 2 years ago

Like how to get the current grid value?

dragontheory commented 2 years ago

Like how to get the current grid value?

No.

bob2517 commented 2 years ago

Could you use CSS variables?

dragontheory commented 2 years ago

Could you use CSS variables?

Yes.

bob2517 commented 2 years ago

You can set CSS variables in ACSS. Would that help? Or would you need to "get" them?

dragontheory commented 2 years ago

You can set CSS variables in ACSS. Would that help? Or would you need to "get" them?

I would need to get them.

bob2517 commented 2 years ago

Ok. Would re-initialising the grid work?