ui: Notifications re-organization/re-style

[johncowen]

This PR re-organizes our global UI notifications slightly:

1. Moves where they appear up to the <App /> component, this solves all sorts of craziness that we've had since the initial v2 rebuild of the UI.
2. Instead of a <Notification /> wrapping component to move whatever you use for a notification up to where they need to appear (via ember-cli-flash), we now use a {{notification}} modifier now we have modifiers.
3. Global notifications/flashes are no longer special styles of their own. You just use the {{notification}} modifier to hoist whatever component/element you want up to the top of the page. This means we can re-use our existing <Notice /> component for all our global UI notifications (this is the user visible change here)

All notifications now look like this i.e. the same as our Notice component

This is only ~80% towards where I want this to end up.

1. I found that we use data-notification for test selecting, this shows how old this stuff is. It should be data-test-notification, and ideally a pageobject. This code would have originated from the first few weeks of the build. For the moment I've centralized this in one place in the app (the modifier) and left it as something I can do in a follow up PR as it will mean a bunch of test selector/page-object wrangling and this PR was already getting a little big.
2. Our models that still don't use our new componentized approach for CRUD have had their Consul::----::Notifications components moved up to <Hashicorp-Consul />. This was just easier for the moment, especially considering the next large chore is probably moving all these to our new style CRUD (getting rid of the old style mixins that are left)

There might be a bit more to add here, so I'll either do that later or inline if I spot anything useful to add. I also split the commits up a little to vaguely describe groups of changes there.

Further notes:

1. We half-used an existing {{style}} modifier which will be useful for us in other places in the UI, but I wanted to be able to delay style modifications easily for animation styling purposes. So I took it and modified it slightly to allow me to do that. I might clean it all up a little further down the line.
2. <AppView /> is now pretty much just a glorified partial. At some point soon I'll probably update this component to be called <Page /> and upgrade it to glimmer/native-named-slots soon. This was one of the large benefits and objectives of doing this overall.

[evrowe]

Mostly just left a few questions; found one typo that needs changing but otherwise this looks good to go

[evrowe]

So if I'm following correctly, what this means is that the modifier allows any arbitrary DOM element, regardless of where it is in the tree, to work as a notification? (Very handy)

[johncowen]

Yup, that was the was the realisation I had also 😸

There's a little gotcha tho 🤫 don't tell anyone 😄 You lose any event handlers or reactiveness when the DOM element is 'hoisted' as it essentially stringifies the DOM and then reinserts it into the DOM via outerHTML. I only sleep well at night as I know we don't need this functionality (yet at least 😬 ). I'm sure there'll be a way around that if/when we do though.

We also have <Portal />s (a 3rd party dep) which does keep all the events/reactivity, but unfortunately we can't use this here as the extra feature we need in this case is for the original DOM to stick around on the page even when the component where it originally took the outerHTML from has gone from the page. Its a fun problem I'd eventually like to fix properly, we can chat a bit more about this later also if you are interested more.

[evrowe]

So then is the use of the style helper as opposed to directly defining the style= attribute to ensure that updates/changes are tracked and applied correctly? Or is it to allow logic and references to template variables to happen more cleanly? Or something else?

[johncowen]

Ah good question! So this one has extra bits. Doing style="color: red;" or as a helper say style={{style-helper (hash color='red')}} means we 'come a cropper' with CSP.

There's a PR here that is related to that overall CSP thing, and I have a long term 'project' to solve this.

The style modifier (which modifies the DOM element) uses imperative-like DOM interfaces to modify the element directly instead of using a declarative template (HTMLs style=""). This means that we don't get caught out by CSP.

There are a few places in the UI where this modifier can be used, and here was the time where I really, properly needed it, instead of it being just an improvement towards a longer term task.

Funnily enough this is one of the places where now in hindsight I was definitely a bit blinkered. At the time I had the choice of using the 3rd party style modifier but went with my own in order to add an extra delay, foregoing the benefits of the dependency.

But now that its sat a little I've realised it would be better to tease apart the delay functionality so it can be used with both {{style}} and a new {{class-map}} for animating using classes as well as styles. Happy to leave as is now though and sort that some other time (but before we continue using the style modifier in more than one place)

(sorry bit long that one ^ 😆 Let me know if you have any other q's from that)

[johncowen]

I added a GH suggestion here (which I'll accept/commit now) for the typo, plus some answers to your queries. So I think its good to go, unless we want to do the {{component}} thing also, which is fine by me if we do. Lets touch on this PR later.

[johncowen]

Ah good question! So this one has extra bits. Doing style="color: red;" or as a helper say style={{style-helper (hash color='red')}} means we 'come a cropper' with CSP.

There's a PR here that is related to that overall CSP thing, and I have a long term 'project' to solve this.

The style modifier (which modifies the DOM element) uses imperative-like DOM interfaces to modify the element directly instead of using a declarative template (HTMLs style=""). This means that we don't get caught out by CSP.

There are a few places in the UI where this modifier can be used, and here was the time where I really, properly needed it, instead of it being just an improvement towards a longer term task.

Funnily enough this is one of the places where now in hindsight I was definitely a bit blinkered. At the time I had the choice of using the 3rd party style modifier but went with my own in order to add an extra delay, foregoing the benefits of the dependency.

But now that its sat a little I've realised it would be better to tease apart the delay functionality so it can be used with both {{style}} and a new {{class-map}} for animating using classes as well as styles. Happy to leave as is now though and sort that some other time (but before we continue using the style modifier in more than one place)

(sorry bit long that one ^ 😆 Let me know if you have any other q's from that)

[johncowen]

Yup, that was the was the realisation I had also 😸

There's a little gotcha tho 🤫 don't tell anyone 😄 You lose any event handlers or reactiveness when the DOM element is 'hoisted' as it essentially stringifies the DOM and then reinserts it into the DOM via outerHTML. I only sleep well at night as I know we don't need this functionality (yet at least 😬 ). I'm sure there'll be a way around that if/when we do though.

We also have <Portal />s (a 3rd party dep) which does keep all the events/reactivity, but unfortunately we can't use this here as the extra feature we need in this case is for the original DOM to stick around on the page even when the component where it originally took the outerHTML from has gone from the page. Its a fun problem I'd eventually like to fix properly, we can chat a bit more about this later also if you are interested more.

[johncowen]

FYI: theres already a different plan to re-assess this in an un-blinkered state.

P.S. Not now or here, just at some point soon, before its usage spreads anywhere else.

[hc-github-team-consul-core]

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/508854.