Add API notification support to new dashboard

[agjohnson]

I started with a KO view for notifications, and the amount of boilerplate was fairly heavy, as observables are needed for each notification attribute.

This is a quick test of using Lit for digestible chunks of HTML. I will say it's way nicer than using KO views.

I'm wondering if it's worth testing Lit in this application. We're gaining more knowledge on web components, and this is one of the use cases that seems like a good fit.

• Refs https://github.com/readthedocs/ext-theme/issues/259

Work left here is all pretty minor:

• [x] Support build.error in the notification list when it's presented by the API
• [x] Clean up of FUI customizations, maybe set this up as a separate variant or maybe just see how much we've used .ui.message first of all.
• [x] Clean up some of the KO view leftovers as I've made changes as I moved through this
• [x] Come back to deal with https://github.com/readthedocs/ext-theme/issues/313
• [x] Fix build detail notification extraneous polling issues
• [x] Fix notification overpolling issues with web component
• [x] Support query by state__in filter

[agjohnson]

@humitos I am putting this example because I wanted to see what you thought here. I can push up the basic KO view here too if you'd like comparison on the two approaches.

There are some downsides to this approach I feel -- mainly that this moves HTML templating into JS, which is harder.

However, there are no more KO data bindings to be aware of, it's just straight JS string/HTML template interpolation.

I will say it felt way easier to author this as a web component. If it feels like an upgrade, I feel it's worth testing the pattern a bit more and taking on Lit as a dependency (even if at least just temporarily).

[agjohnson]

I'm pretty biased on this and it's hard to not be subjective here.

Hah, yeah. I would say this is objectively better overall :smile:

The places that I feel like this pattern is not obvious yet are:

• Where we have complex KO views, like the BuildDetailView. There is a lot of nested HTML in the KO view, I don't yet know how a web component pattern would fit in here. I think a web component could replace this, but I don't yet know of the pattern.
• And where we are mixing KO views and web components (temporarily?). In this PR, this is in the build detail view where a big KO view and a web component are mixed. Some logic needs to happen at the KO view, and the web component does conflict with that. Or it doesn't share a connection to the KO view.

I am going to experiment really quick with the second issue there on the build detail page. But if I don't make quick progress here, the build detail page might show notifications through a KO view instead.

This PR doesn't need to be blocked by using Lit, but I feel it's a pretty safe/obvious/isolated/low stakes place to experiment with moving towards Lit.

[agjohnson]

A pattern to make a hybrid KO view and Lit web component is actually rather straight forward:

<rtd-notification-list
  url="{% url "projects-builds-notifications-list" project.slug build.pk %}"
  csrf-token="{{ csrf_token }}"
  data-bind="webcomponent: {finished: !isPolling()}">
</rtd-notification-list>

This is a (custom) KO webcomponent data binding that applies the inverse of the observable isPolling to the web component property finished.

So, I think I proceed and the first use case needs some more thought, but I am not prioritizing replacing big KO views like that yet.

[humitos]

Wow 😲! It looks pretty straightforward. Is this a pattern that you designed or it's a standard way to set webcomponent attributes?

[agjohnson]

This is not standard, but it was also really similar to the other KO plugins in this repo. I reused the semanticui data binding plugin basically, just stripped down. I did see an old NPM package, and didn't set it up correctly while testing I eventually found, but by that time had a working plugin anyways.

I'll learn more while I play with it today, but the nice thing is that we can pass in whole data objects -- as opposed to passing in individual strings/booleans as you would normally do with web components. This feels like it will be really easy to integrate the two and slowly move towards Lit.

[agjohnson]

I am on to cleaning things up, but I am rather happy with the end result so far. Here is what the notification messages look like at the top of the page currently:

(Note this is just hacked HTML, if this were 5 separate messages the icons/text would be different)

The order of notifications here are: error, warning, info, note, tip. Note and tip notifications don't use the inverted styling as a way to appear less visually important - for optional tasks, where error/warning/info are for mandatory tasks/information.

On the build detail view, where the background in the area for errors is already inverted, note and tip have inverted styles:

I'm not yet settled if this should also replace the default .ui.message styles (which I don't care for). I might start this off as a separate .ui.message variant so that we don't override the places we have already used .ui.message to add supporting prose to forms/etc.

The style changes here are:

• Message is single line to make for a more compact display
• Header is inline, but bold to stand out
• Link color matches the text color while inverted, instead of being link teal

Work left here is all pretty minor

[humitos]

On the build detail view, where the background in the area for errors is already inverted, note and tip have inverted styles:

I would use always the same rule no matter what page we are. Otherwise the thought behind the pattern becomes unnecessary complex and harder to understand for users.

The style changes here are:

Why not using the same style everywhere for simplification?

The style that have the header separated from the body looks pretty good --like the one you shared before in https://usercontent.irccloud-cdn.com/file/WLO4Qb75/image.png

Also, it seems these matches other "in page" notifications we are showing to users on settings pages and similar. IMHO, it makes sense to re-use these style as much as we can instead of creating a specific style for each of these notifications. From a user perspective will be easier to parse and for us to maintain the code.

[agjohnson]

Thanks. There is a lot to cover here on why these changes are needed and I don't know if it help to go through all of this process again. Overall, I don't want to get too stuck on the visual styles here though, I feel it's better to tune as we go, and the main reasoning is allow us to explore notifications more.

Otherwise the thought behind the pattern becomes unnecessary complex and harder to understand for users.

Users aren't going to know what tip or note blocks mean by the colors used either way, so this alone is not going to confuse users.

Why not using the same style everywhere for simplification?

Tip and note use a light background to help communicate to the user that they don't require attention. This is opposed to error, warning, and info, which stand out and pull the users attention away from everything else on the dashboard. The user should feel they need to do something with these immediately.

We are not using tip or note level messages yet either way however.

Tip level messages to me would be "You are using our beta" or "Have you tried Addons?". These styles allow for us to experiment with adding more frequent tip/note notifications without visually overwhelming the dashboard and user.

Also, it seems these matches other "in page" notifications we are showing to users on settings pages and similar. IMHO, it makes sense to re-use these style as much as we can instead of creating a specific style for each of these notifications.

I noted this above, this is already the end goal. I am isolating the work here to just notifications at the moment. Eventually the default .ui.message styles will be replaced with the light colored message here though. The reasons for this are:

• Default .ui.message styles feel out of place and have bugs
  • Styles don't match visually with other components like .ui.toast, .ui.segment, etc
  • They use colors not used anywhere else in the theme
  • Contrast is too low for accessibility standards on most variations
  • Link colors do not match the message body color

The style that have the header separated from the body looks pretty good --like the one you shared before in https://usercontent.irccloud-cdn.com/file/WLO4Qb75/image.png

In addition to the reasons above regarding .ui.message, the reason the default styles need fixes here are:

• The rounded corners on messages. There is no middle attached variant, so there are always rounded corners, which is sloppy for an inline component:

• The header line separated takes up a lot of room and leaves a lot of dead space in the message. Space is already at a premium in the build detail view and at the top of the page where the more space we use the more likely we'll drastically shift the layout after partially loading the view.
• The pastel colors on default messages look the most out of place here.

Anyways, this is all just background. Most important here is that the style changes here are quite minimal. I feel it's best to move forward and tune these as we actually start using things like tip/note messages.

[humitos]

I don't want to get too stuck on the visual styles here though, I feel it's better to tune as we go, and the main reasoning is allow us to explore notifications more

I don't want to block this work on this at all. I was just providing some feedback on what I saw without the intention of blocking it since the important piece here is to discuss about is the pattern itself, not the styling.

That said, thanks for all the explanation about the style and the reasons behind them 👍🏼

[agjohnson]

These tip/note notifications and styles will feel a bit new and out of place until we are using them more, and certainly something we can tweak as we go.

I've put more background on the continuation of this style work for other instances of .ui.message here:

• 316

This would be something to chip away at later, it's not a strong priority.

[agjohnson]

Here's a wrap up of all of the various instances of notifications, dismissable and permanent.

Screencast from 2024-03-25 16-34-11.webm

[humitos]

I'm merging changes from main and rebuilding the assets. I hope I'm doing this correctly 🙃

[agjohnson]

Things look good, including built assets. Thanks for taking a look through this!