What does this change do?
This change improves the `upload_theme` system test helper method by
automatically setting the uploaded theme as the default theme for the
site. This is to make it easier for users to use the theme instead of
having to fiddle with theme previews. The default behaviour of setting
the uploaded theme as the site's default theme can be disabled by
passing `false` to the `set_theme_as_default` keyword argument.
This change also introduces a new `upload_theme_component` system test
helper method for uploading theme components. The difference between the
`upload_theme` helper method is that the theme component is
automatically added to the site's default theme when uploaded. The theme
which the theme component is added to can be configured via the
`parent_theme_id` keyword argument.
For both methods, we also no longer require the path to the theme to be
provided. Instead both methods will look through the callstack and can
figure out the theme's directory based on the convention that the
theme's system tests are placed in the `spec/system` directory of the
theme folder. This change simplifies the usage of the methods for users
and helps to remove code like `upload_theme_component(File.expand_path("../..", __dir__))`.
As per 92839dc722 lastMessage won't be null when a message is deleted. This would cause sorting issues when messages from a direct message channels are deleted as we would try to sort on last message and thought it would exist when actually it would be a null message.
I might rethink this design to not return any last_message in this case soon as checking on ID feels dirty and prone to error, so only fixing the issue for now.
This commit is also using `@cached` to avoid replaying the sort logic on each access.
Discourse has a custom stylesheet pipeline which compiles things 'just in time'. The only place we were still running sass files through sprockets was for the `/tests` route in development mode. This use can be removed by compiling the relevant stylesheets through ember-cli instead (which we were already doing for testem runs)
This work was prompted by the incompatibility of dartsass-sprockets with the latest sass-embedded release (https://github.com/tablecheck/dartsass-sprockets/issues/13)
Our custom implementation of `getOwner` includes a fallback which returns an owner, even if the passed object does not have one set. This is confusing and creates a false sense of security. Generally if the fallback is used, it means there is a problem with the patterns being used.
This commit renames our custom implementation to `getOwnerWithFallback`, while maintaining the old `getOwner` export with a deprecation notice. Core code is updated to use the official `@ember/application` implementation, or the new `getOwnerWithFallback` function.
This commit updates all core uses of `{ getOwner } from discourse-common/lib/get-owner` to use `getOwnerWithFallback`. Future commits will work through and convert many of these to use the official `@ember/application` implementation
A new `rawRenderGlimmer` function is introduced which can be used to render glimmer components inside our legacy 'raw hbs' views. See discourse/lib/raw-render-glimmer for more information. This will help as we work to move away from raw-hbs use.
This PR introduces three new concepts to Discourse codebase through an addon called "FloatKit":
- menu
- tooltip
- toast
## Tooltips
### Component
Simple cases can be express with an API similar to DButton:
```hbs
<DTooltip
@Label={{i18n "foo.bar"}}
@ICON="check"
@content="Something"
/>
```
More complex cases can use blocks:
```hbs
<DTooltip>
<:trigger>
{{d-icon "check"}}
<span>{{i18n "foo.bar"}}</span>
</:trigger>
<:content>
Something
</:content>
</DTooltip>
```
### Service
You can manually show a tooltip using the `tooltip` service:
```javascript
const tooltipInstance = await this.tooltip.show(
document.querySelector(".my-span"),
options
)
// and later manual close or destroy it
tooltipInstance.close();
tooltipInstance.destroy();
// you can also just close any open tooltip through the service
this.tooltip.close();
```
The service also allows you to register event listeners on a trigger, it removes the need for you to manage open/close of a tooltip started through the service:
```javascript
const tooltipInstance = this.tooltip.register(
document.querySelector(".my-span"),
options
)
// when done you can destroy the instance to remove the listeners
tooltipInstance.destroy();
```
Note that the service also allows you to use a custom component as content which will receive `@data` and `@close` as args:
```javascript
const tooltipInstance = await this.tooltip.show(
document.querySelector(".my-span"),
{
component: MyComponent,
data: { foo: 1 }
}
)
```
## Menus
Menus are very similar to tooltips and provide the same kind of APIs:
### Component
```hbs
<DMenu @ICON="plus" @Label={{i18n "foo.bar"}}>
<ul>
<li>Foo</li>
<li>Bat</li>
<li>Baz</li>
</ul>
</DMenu>
```
They also support blocks:
```hbs
<DMenu>
<:trigger>
{{d-icon "plus"}}
<span>{{i18n "foo.bar"}}</span>
</:trigger>
<:content>
<ul>
<li>Foo</li>
<li>Bat</li>
<li>Baz</li>
</ul>
</:content>
</DMenu>
```
### Service
You can manually show a menu using the `menu` service:
```javascript
const menuInstance = await this.menu.show(
document.querySelector(".my-span"),
options
)
// and later manual close or destroy it
menuInstance.close();
menuInstance.destroy();
// you can also just close any open tooltip through the service
this.menu.close();
```
The service also allows you to register event listeners on a trigger, it removes the need for you to manage open/close of a tooltip started through the service:
```javascript
const menuInstance = this.menu.register(
document.querySelector(".my-span"),
options
)
// when done you can destroy the instance to remove the listeners
menuInstance.destroy();
```
Note that the service also allows you to use a custom component as content which will receive `@data` and `@close` as args:
```javascript
const menuInstance = await this.menu.show(
document.querySelector(".my-span"),
{
component: MyComponent,
data: { foo: 1 }
}
)
```
## Toasts
Interacting with toasts is made only through the `toasts` service.
A default component is provided (DDefaultToast) and can be used through dedicated service methods:
- this.toasts.success({ ... });
- this.toasts.warning({ ... });
- this.toasts.info({ ... });
- this.toasts.error({ ... });
- this.toasts.default({ ... });
```javascript
this.toasts.success({
data: {
title: "Foo",
message: "Bar",
actions: [
{
label: "Ok",
class: "btn-primary",
action: (componentArgs) => {
// eslint-disable-next-line no-alert
alert("Closing toast:" + componentArgs.data.title);
componentArgs.close();
},
}
]
},
});
```
You can also provide your own component:
```javascript
this.toasts.show(MyComponent, {
autoClose: false,
class: "foo",
data: { baz: 1 },
})
```
Co-authored-by: Martin Brennan <mjrbrennan@gmail.com>
Co-authored-by: Isaac Janzen <50783505+janzenisaac@users.noreply.github.com>
Co-authored-by: David Taylor <david@taylorhq.com>
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
favicons were removed in #17477
theme-preview appears to once be a `.hbs` file shared by several
components, which went away with the refactor in #20282. It must
have accidentally came back with some rebase error, and then got
picked up by the template-only component codemod. It's unused and
does nothing anyway.
It seems like the intention is to update the tab selection at the bottom when the scrollable pane changes enough. In my testing (and I think by definition?), it doesn't seem like `scrollLeft` ever exceeds `offsetWidth`, so that tab-switching behavior doesn't ever happen
What is the problem we are trying to solve here?
The `/` path in our Ember app leads to the `discovery.index` route but
we actually don't render anything on that route leading to a blank page
if the Ember app were to transition to it which is what was happening
when a user adds a custom sidebar section link with the `/` path.
What is the fix there?
Instead of generating a link for the `discovery.index` route when
creating the sidebar section link, we detect if the Ember route is
`discovery.index` and change it to the `discovery.${defaultHomepage()}`
route instead.
Why this change?
Before this change, we were doing a partial match when checking for
existence. This is a source of flaky tests because a chat message with
text `this is a message` will match any substring like `message` or `a`.
What does this change do?
This change removes the partial match and instead opts for the
`exact_text` option instead.
Some sites have a large number of categories and fetching the category
IDs or category topic IDs just to build another query can take a long
time or resources (i.e. memory).
Our existing PluginOutlet system allows theme/plugin developers to easily insert new content into Discourse.
Another common requirement is to **replace** existing content in Discourse. Previously this could be achieved either using template overrides, or by introducing new content via a PluginOutlet and then hiding the old implementation with CSS. Neither of these patterns are ideal from a maintainability or performance standpoint.
This commit introduces a new mode for PluginOutlets. They can now be used to 'wrap' blocks of content in core. If a plugin/theme registers a connector for the outlet, then it will be rendered **instead of** the core implementation. If needed, outlets can use `{{yield}}` to render the core implementation inside their own implementation (e.g. to add a wrapper element).
In this 'wrapper' mode, only one connector can be registered for each outlet. If more than one is registered, only one will be used, and an error will be printed to the console.
To introduce a new PluginOutlet wrapper, this kind of thing can be added to a core template:
```hbs
<PluginOutlet @name="site-logo" @defaultGlimmer={{true}} @outletArgs={{hash title=title}}>
<h1>This is the default core implementation: {{title}}</h1>
</PluginOutlet>
```
A plugin/theme can then register a connector for the `site-logo` outlet:
```hbs
{{! connectors/site-logo/my-site-logo-override.hbs }}
<h2>This is the plugin implementation: {{@outletArgs.title}}</h2>
```
Care should be taken when introducing new wrapper PluginOutlets. We need to ensure that
1) They are properly sized. In general it's preferable for each outlet to wrap a small amount of core code, so that plugin/themes only need to re-implement what they want to change
2) The `@outletArgs` are carefully chosen. It may be tempting to pass through lots of core implementation into the outletArgs (or worse, use `this` to pass a reference to the wrapping component/controller). Doing this will significantly increase the API surface area, and make it hard to refactor core. Instead, we should aim to keep `@outletArgs` to a minimum, even if that means re-implementing some very simple things in themes/plugins.
Previously we were using 'mouseup', which meant that if you started the click inside, and then dragged to outside the modal, it would still close. This kind of dragging action is common when selecting text, and having it close the modal can be very frustrating.
Simply switching to a 'click' listener doesn't totally solve the problem, because when a click event involves dragging from one element to another, the browser will fire the event on "the most specific ancestor element that contained both elements". For modals, the most specific common ancestor was still the `modal-middle-container`, which would cause the modal to close.
Therefore, this commit sets the modal containers to have `pointer-events: none`, and sets up the click listener on the `.modal-backdrop` element, which is **adjacent** to the modal in the DOM. That means that click events fired on any ancestors of the modal will not accidentally trigger closure.
This would cause an error when deleting the original message of a thread, due to the non existing `last_message`. This fix is implemented using the null pattern.
Note this commit is also using this opportunity to unify naming of null objects, `Chat::DeletedUser` becomes `Chat::NullUser`, it feels better to have a name describing what is the object, instead of a name describing why this object has to be used, which can change depending on cases.
This is an aesthetic change. Currently, if one of the scores involved in the reviewable score explanation is negative, we display it as: + -value. This changes that.
I also made an attempt at converting the component into GJS format. This is done as a separate commit.
Alan Guo Xiang Tan
Krzysztof Kotlarek
Kris
Joffrey JAFFEUX
David Taylor
Jarek Radosz
Arpit Jalan
dependabot[bot]
Godfrey Chan
Bianca Nenciu
Renato Atilio
Ted Johansson