Prior to this fix clicking outside text and reseting the selection wouldn't clear the quote state, which would cause a click on "reply" or "create" to start the composer with the quote state.
This commit attempts to simplify this behaviour by not mutating quote state while the menu is opened. The quote state will now only be cleared when the menu is closed.
No tests have ever been written for this complex and subtle behavior (both `mousedown` and `selectionchange` events can trigger the final `selectionChanged` codepath which prevents us to for example stop the event when clicking quote as it will still change the selection even if we can prevent the `mousedown`. Ideally a huge part of this code should be rewritten to be easier to test, this commit only attempt to fix a regression introduced when using FloatKit to position the menu.
Currently moderators can see the custom public sidebar section edit button, but they are prevented from making any changes by an error. According to the back-end, moderators can not access these.
This PR hides the custom public sidebar section edit button, as well as the "make public" checkbox of the create modal, if the user is not an admin, bringing the UI in line with the back-end.
If needed, we can add a site setting to allow moderator access when the need arises.
JS tests expect `show_copy_button_on_codeblocks` to be false (because
default before #81f3f56 was false). There is probably a different
issue at play here with JS tests, I haven't dug into it yet.
Instead, this PR adds a system test to ensure copy button is present
for code blocks with default site settings enabled.
1. actually call `popupAjaxError`, thanks :P
2. don't close a modal on error
3. use `extractError()` instead of manually joining error messages
4. …or passing just the error object to `this.flash`
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.
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 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.
