This hook is `cancel()`'d in a willTransition hook, but that isn't always enough. It might still be scheduled if there is a scroll event between `willTransition`, and the transition actually completing. Following c2d94be06e, this kind of scroll event happens when the loading indicator is set to 'spinner'. This would put the router in a weird state and cause navigation issues.
Also takes the opportunity to remove JQuery from this code path
https://meta.discourse.org/t/286463/15
If a group is < 5 members, the mention warning doesn't need to
be so harsh. This commit changes the copy for the existing warning
and adds a new one for groups that are >= 5 members.
When going 'back', default browser behavior is to restore the scroll position. Unfortunately sites are given no control over the timing of this restoration, which means it can happen halfway through an Ember transition. Therefore we disable it, and re-implement the functionality in our scroll-manager service.
We inadvertently dropped this configuration in 7c9cf666da, which led to issues like https://meta.discourse.org/t/286463
Making the icons available generally in tests is tricky because they're generated dynamically by the rails server. However, if we restrict it to dev-mode (`/tests` in a browser) then it's possible to load them from the running rails server. This is purely a visual thing to make debugging easier - it should not affect test behavior.
Reverts
- DEV: maxmind license checking failing tests #24534
- UX: Show if MaxMind key is missing on IP lookup #18993
These changes are leading to surprising results, our logs are now filling up with warnings on dev environments
We need the change to be redone
+ native classes
+ tracked properties
- Ember.Object
- Ember.Evented
- observers
- mixins
- computed/discourseComputed
Also removes unused wizard infrastructure for warnings. It appears
that once upon on time, either the server can generate warnings,
or some client code can generate them, which requires an extra
confirmation from the user before they can continue to the next step.
This code is not tested and appears unused and defunct. Nothing
generates such warning and the server does not serialize them.
Extracted from https://github.com/discourse/discourse/pull/23678
Followup to e37fb3042d
Some plugins like discourse-ai and discourse-saml do not
nicely change from kebab-case to Title Case (e.g. Ai, Saml),
and anyway this method of getting the plugin name is not
translated either.
Better to use the plugin setting category if it exists,
since that is written by a human and is translated.
This commit extracts the storage part of the route-scroll-manager into a dedicated service. This provides a key/value store which will reset for each navigation, and restore previous values when the user uses the back/forward buttons in their browser.
This gives us a reliable replacement for the old `DiscourseRoute.isPoppedState` function, which would not work under all situations.
Previously reverted in e6370decfd. This version has been significantly refactored, and includes an additional system spec for the issue we identified.
In the past, our loading spinner implementation used Ember's loading substate. That meant that, when the site setting was toggled, there would be fundamental changes in the routing behavior.
This commit simplifies things so that the (non-default) loading spinner implementation is purely a styling thing, and behaves exactly the same as the spinner which appears under the 'slider' configuration when loading takes too long.
This does involve a slight UX change. Now, the entire page will be replaced by a loading spinner instead of just the relevant `{{outlet}}`. We strongly recommend sites use the new default 'slider' behavior.
* Remove checkmark for official plugins
* Add author for plugin, which is By Discourse for all discourse
and discourse-org github plugins
* Link to meta topic instead of github repo
* Add experimental flag for plugin metadata and show this as a
badge on the plugin list if present
---------
Co-authored-by: chapoi <101828855+chapoi@users.noreply.github.com>
This commit makes it so the fullscreen code modal grows
to fit its content, and doesn't show horizontal scrollbars
unless the entire screen is filled by the modal already.
The code syntax highlighting and copy buttons were also
broken in fullscreen because of modal changes over time.
This reverts commit 20e562bd99, 161256eef8 and a8292d25f8.
It looks like this affected cache-restoration of topic lists in some circumstances. It also looks like routing behavior may vary when toggling the loading indicator between spinner and slider.
More investigation and testing required.
For transitions to nested routes (e.g. /u/blah/activity), where each layer has an async model hook, the `loading` event will be fired twice within the same transition. This was causing the loading slider to jump backwards halfway through loading. This commit fixes things to handle nested loading events with a single animation.
The old heuristic was 'a transition to a URL (i.e. not a named route) which was not triggered by DiscourseURL'. That logic is flawed now that we're increasingly using Ember's routing methods.
This commit extracts the storage part of the route-scroll-manager into a dedicated service. This provides a key/value store which will reset for each navigation, and restore previous values when the user uses the back/forward buttons in their browser.
Should fix https://meta.discourse.org/t/-/285768.
Appending without cloning was causing the item to be removed from the
DOM but on a 1-item grid we skip the rest of the grid's rendering,
hence the item was never re-inserted. Cloning ensures we don't remove
the item during processing (it does get removed later on when rendering
the grid's columns).
Moves the patch from ember-source to ember-cli so that it's easier for us to feature-flag an ember-source upgrade without fighting with patch-package. We'll be able to remove this patch once we're fully on Ember 5.x.
(ref https://github.com/discourse/discourse/pull/21720)
Having async cleanup on a modifier is problematic because it means it might persist beyond the end of a test, leading to flaky 'Test is not isolated' errors.
Discourse already includes version information in a `<meta` tag on the page. This commit surfaces it to the console on boot for easier access, and also adds the Ember version (which will be particularly useful as we start rolling out the upgrade to Ember 5)
This was accidentally selecting the close button on `<DModalLegacy />`, which is present in the DOM with `display: none`. The close button logic would close any active modal, so the test would pass. However, it will stop passing when we remove the legacy modal system.
In the long term we should aim to modernize these places, but for now this change will make them compatible with Ember 5.x (while maintaining compatibility with Ember 3.28)
This commit adds a new `search_default_sort_order` site setting,
set to "relevance" by default, that controls the default sort order
for the full page /search route.
If the user changes the order in the dropdown on that page, we remember
their preference automatically, and it takes precedence over the site
setting as a default from then on. This way people who prefer e.g.
Latest Post as their default can make it so.
Why this change?
The test has been flaky on CI with the following assertion failing:
```
not ok 302 Firefox 115.0 - [899 ms] - Browser Id 5 - Acceptance: Fast Edit: Works with keyboard shortcut
---
actual: >
Element #fast-edit-input does not exist
expected: >
Element #fast-edit-input exists
```
The hypothesis here is that we are triggering the `E` keypress event
before the `.quote-button` menu has appeared. When that happens, we will
end up opening the composer instead of triggering the fast edit editor.
Why this change?
As the number of themes which the Discourse team supports officially
grows, we want to ensure that changes made to Discourse core do not
break the plugins. As such, we are adding a step to our Github actions
test job to run the QUnit tests for all official themes.
What does this change do?
This change adds a new job to our tests Github actions workflow to run the QUnit
tests for all official plugins. This is achieved with the following
changes:
1. Update `testem.js` to rely on the `THEME_TEST_PAGES` env variable to set the
`test_page` option when running theme QUnit tests with testem. The
`test_page` option [allows an array to be specified](https://github.com/testem/testem#multiple-test-pages) such that tests for
multiple pages can be run at the same time. We are relying on a ENV variable
because the `testem` CLI does not support passing a list of pages
to the `--test_page` option.
2. Support a `/testem-theme-qunit/:testem_id/theme-qunit` Rails route in the development environment. This
is done because testem prefixes the path with a unique ID to the configured `test_page` URL.
This is problematic for us because we proxy all testem requests to the
Rails server and testem's proxy configuration option does not allow us
to easily rewrite the URL to remove the prefix. Therefore, we configure a proxy in testem to prefix `theme-qunit` requests with
`/testem-theme-qunit` which can then be easily identified by the Rails server and routed accordingly.
3. Update `qunit:test` to support a `THEME_IDS` environment variable
which will allow it to run QUnit tests for multiple themes at the
same time.
4. Support `bin/rake themes:qunit[ids,"<theme_id>|<theme_id>"]` to run
the QUnit tests for multiple themes at the same time.
5. Adds a `themes:qunit_all_official` Rake task which runs the QUnit
tests for all the official themes.
Move external login logic from the **Login Modal** -> **Login Service**. This is advantageous as we can utilize the external login logic from both within and outside of the login modal.
A downside of having the external login logic within the login modal is that there is a brief "flash" of the login modal being rendered and then us automatically redirecting to the external login method. This PR will clean up the visual side affects.
This change means that the `/my` redirects will be handled by the ember 'unknown' route, and will therefore function correctly when using pure-ember transition methods like `router.transitionTo`
We want / to display one of our discovery routes/controllers, but we don't want to register it as `discovery.index` because that would break themes/plugins which check the route name. Previously, this was handled using a variety of approaches throughout the codebase (in discourse-location, discourse-url and mapping-router). But even then, it didn't work consistently. For example, if you used an Ember method like `router.transitionTo("/")`, an empty `discovery.index` page would be rendered.
This commit switches up the approach. `discovery.index` is now defined as a real route, and redirects to the desired homepage. To preserve the `/` as a 'vanity url', we patch the method on the router responsible for persisting URLs to the Ember Router and the browser. The patch identifies a relevant transition by looking for a magic query parameter.
In an ideal world, we wouldn't be patching the router at all. But at least with this commit, the workaround is all in one place, and works consistently for all navigation methods. The new strategy is also much better tested.
When we started using NumberField for integer site settings
in e113eff663, we did not end up
passing down a min/max value for the integer to the field, which
meant that for some fields where negative numbers were allowed
we were not accepting that as valid input.
This commit passes down the min/max options from the server for
integer settings then in turn passes them down to NumberField.
c.f. https://meta.discourse.org/t/delete-user-self-max-post-count-not-accepting-1-to-disable/285162
Why this change?
This test has been flaky on CI: https://github.com/discourse/discourse/actions/runs/6880353258/job/18714366795
However, the way the current assertions are written does not really
allow us to easily figure out what went wrong since we only know that
`#post_4` was not selected. It will be useful to know what was selected
instead of `#post_4` when the test fails.
What does this change do?
This change updates the assertion of the flaky test to reveal which post
was selected should the test fail.
This discourse-common decorator was dependent on the core app, hence creating a circular reference that was breaking the embroider upgrade. (see: #24391)
Raised in https://meta.discourse.org/t/keyboard-navigation-messes-up-the-search-menu/285405
We were incorrectly accessing the highlighted search result target's href which caused issues when navigating the topic list (eg /latest) with **j / k** and then immediately after accessing the search menu and navigating to and selecting a search result with the keyboard.
### Current Behavior
Hitting enter on a search result redirects to the href of the topic in the topic list that was previously highlighted.
### Expected Behavior
Hitting enter on a search result redirects to the href of the highlighted search result.
The default for webpack is to keep cached values indefinitely. In discourse, this unbound memory usage causes node to raise an OOM error after 50-100 rebuilds in development mode (with source maps enabled). Setting maxGenerations=1 means that the cache will be cleaned up regularly. With this change, I see no discernible increase in memory after 150+ rebuilds.
Previously, the discourse-hbr plugin took the entire app tree as its input, and the result would then be merged into the app. This is wasteful and more likely to cause problems in the build pipeline.
See also https://github.com/discourse/discourse/pull/24376
Ember-cli has built-in error pages when there is a build error. Previously these were not being used in Discourse because our custom proxy middleware was too early in the stack. This commit reorders things so that the "broccoli-watcher" middleware runs before our custom proxy. It also disables the `historySupportMiddleware`, which doesn't make sense in our 'always proxy' setup.
This PR refactors the following:
* leaving all the CSS applied to the old `modal-body` classes in their respective files
* made new clean styling for `.d-modal` and refactored the template to use the new BEM classes
* `inner-`, `middle-`, `outer-` container classes are gone and replaced with simplified `wrapper` and `container` classes
* use standardised max-sizes with modifiers `-large` and `-max`
* lighter backdrop,
* min-width to prevent puny modals
* other styling changes regarding padding, close button,…
* pulled out all modal overrides into a general `modal-overrides` file + cleanup of outdated CSS
* pulled out login and create account modal styling into their own file, cause it's such a big override
* removed old general login.scss file for mobile & desktop
* only kept some remainders I don't want to touch in `app/assets/stylesheets/common/base/login.scss`
Previously this was being handled in two places:
1. As a monkey-patch to the Ember router. This would 'trick' the router into rendering a different route, but would leave the browser URL bar unchanged. Many possible bugs can come from this state
2. In the DiscourseURL.routeTo function. This functioned fine as a redirect, but wouldn't have any effect when the transition is handled by Ember
This commit refactors things so that the DiscourseURL redirects are handled the same as our permalinks. When the Ember 'unknown' route is hit, we check for a possible rewrite and redirect there. This is a supported way of doing things, and should be more robust going forwards.
Previously we had similar logic in two places:
1. A DiscourseURL rewrite, based on a site setting
2. Some logic in the user-index route
This commit moves everything into (2) to make things clearer and more consistent
We ask users to confirm their session if they are making a sensitive
action, such as adding/updating second factors or passkeys. This
commit adds the ability to confirm sessions with passkeys as an option
to the password confirmation.
This allows outlets for the post-text-selection-toolbar to
get just the raw markdown of the selected text for a quote,
rather than opening the composer.
This commit changes some plugin outlets in `<Discovery::Layout>`, `<Discovery::Navigation>` and `Discovery::Topics` to improve compatibility with existing customization, simplifying the migration process to the new discovery routes.
In these components, the standard plugin outlets will receive by default at least the arguments: `category` and `tag`.
Furthermore, two new wrapping plugin outlets were added to enable the conversion of existing template overrides to the new pattern: `discovery-list-area` and `topic-list-bottom`. The new template overrides will receive a `model` argument containing the full model handled by the route.
---------
Co-authored-by: David Taylor <david@taylorhq.com>
Follow-up to #24278 that is slightly less trivial.
* Some were "trivial" usages that were missed in the previous PR because the same file that had at least one other non-trivial usage.
* These involve extra arguments or inheritance but I have checked that they seem correct.
- Remove vendored copy
- Update Rails implementation to look for language definitions in node_modules
- Use webpack-based dynamic import for hljs core
- Use browser-native dynamic import for site-specific language bundle (and fallback to webpack-based dynamic import in tests)
- Simplify markdown implementation to allow all languages into the `lang-{blah}` className
- Now that all languages are passed through, resolve aliases at runtime to avoid the need for the pre-built `highlightjs-aliases` index
Previously, the app HTML served by the Ember-CLI proxy was generated based on a 'bootstrap json' payload generated by Rails. This inevitably leads to differences between the Rails HTML and the Ember-CLI HTML.
This commit overhauls our proxying strategy. Now, we totally ignore the ember-cli `index.html` file. Instead, we take the full HTML from Rails and surgically replace script URLs based on a `data-discourse-entrypoint` attribute. This should be faster (only one request to Rails), more robust, and less confusing for developers.
This updates the behaviour to match ember-cli-htmlbars, and should take care of the handful of themes which were relying on runtime compilation in tests (see 4425e99bf9)
This adds the ability to collect stats without exposing them
among other stats via API.
The most important thing I wanted to achieve is to provide
an API where stats are not exposed by default, and a developer
has to explicitly specify that they should be
exposed (`expose_via_api: true`). Implementing an opposite
solution would be simpler, but that's less safe in terms of
potential security issues.
When working on this, I had to refactor the current solution.
I would go even further with the refactoring, but the next steps
seem to be going too far in changing the solution we have,
and that would also take more time. Two things that can be
improved in the future:
1. Data structures for holding stats can be further improved
2. Core stats are hard-coded in the About template (it's hard
to fix it without correcting data structures first, see point 1):
63a0700d45/app/views/about/index.html.erb (L61-L101)
The most significant refactorings are:
1. Introducing the `Stat` model
2. Aligning the way the core and the plugin stats' are registered
This removes all trivial usages of the `{{action}}` keyword (the helper form, not the modifier form), where trivial means:
1. It's a co-located component (`.hbs` next to `.js`)
2. The JS file has a default export that is native class
3. `{{action "foo"}}` or `(action "foo")` with no extra arguments
4. There is a corresponding `foo()` method defined on the class (not inherited, etc)
There are more usages that is slightly more involved (with arguments, etc) that we can deal with, but this PR seems big enough so I just included the easiest cases here.
To aid review, each file is converted in an individual commit, and the matching method is temporary annotated with `@__action__` instead of the normal `@action`. This forces a git diff when it is already annotated as `@action`.
* DEV: {{action}} -> @action admin-penalty-post-action.hbs
* DEV: {{action}} -> @action admin-report.hbs
* DEV: {{action}} -> @action admin-watched-word.hbs
* DEV: {{action}} -> @action emoji-value-list.hbs
* DEV: {{action}} -> @action bool.hbs
* DEV: {{action}} -> @action category.hbs
* DEV: {{action}} -> @action secret-value-list.hbs
* DEV: {{action}} -> @action category-list.hbs
* DEV: {{action}} -> @action color.hbs
* DEV: {{action}} -> @action compact-list.hbs
* DEV: {{action}} -> @action group-list.hbs
* DEV: {{action}} -> @action host-list.hbs
* DEV: {{action}} -> @action named-list.hbs
* DEV: {{action}} -> @action simple-list.hbs
* DEV: {{action}} -> @action tag-group-list.hbs
* DEV: {{action}} -> @action tag-list.hbs
* DEV: {{action}} -> @action value-list.hbs
* DEV: {{action}} -> @action watched-word-form.hbs
* DEV: {{action}} -> @action composer-messages.hbs
* DEV: {{action}} -> @action section.hbs
* DEV: {{action}} -> @action user-status-picker.hbs
* DEV: cleanup @__action__ -> @action
Followup to 545e92039c
This commit fixes an issue where hashtags on user activity stream
items past page 1 did not get decorated. This is because of a bug
in the user stream component, where it was trying to get stream
items to decorate after the AJAX call but before they had been
rendered by Ember. This can be fixed by wrapping this decoration
logic in later() to run on the next runloop.
This commit adds an /admin/customize/theme-components route,
that opens the theme page with the components tab pre-selected,
so people can navigate to that directly.
Switches to using a dialog to confirm a session (i.e. sudo mode for
account changes where we want to be extra sure the current user is who
they say they are) to match what we do with passkeys.
https://github.com/discourse/discourse/pull/22622 accidentally introduced an `@action` decorator inside the actions hash, which does not work. This commit modernizes the component by removing the actions hash altogether.
Previously, we were parsing webpack JS chunk filenames from the HTML files which ember-cli generates. This worked ok for simple entrypoints, but falls apart once we start using async imports(), which are not included in the HTML.
This commit uses the stats plugin to generate an assets.json file, and updates Rails to parse it instead of the HTML. Caching on the Rails side is also improved to avoid reading from the filesystem multiple times per request in develoment.
Co-authored-by: Godfrey Chan <godfreykfc@gmail.com>
Followup to b53449eac9,
it was too easy to add broken routes which would break
configuration for the whole site, so now we validate ember
routes on save.
In some browsers, 2FA login was failing with a "request is already
pending" error. This only applied when `experimental_passkeys` was
enabled and on Chrome only. This was due to the fact that the webauthn
API only supports one auth attempt at a time, so the security key call
needs to abort the passkey's conditional UI request before starting.
I am not sure we can test this. We have system specs that simulate
webauthn credentials and they didn't catch this (probably because the
simulation covers the whole flow).
With Embroider, we can rely on async `import()` to do the splitting
for us.
This commit extracts from `pretty-text` all the parts that are
meant to be loaded async into a new `discourse-markdown-it` package
that is also a V2 addon (meaning that all files are presumed unused
until they are imported, aka "static").
Mostly I tried to keep the very discourse specific stuff (accessing
site settings and loading plugin features) inside discourse proper,
while the new package aims to have some resembalance of a general
purpose library, a MarkdownIt++ if you will. It is far from perfect
because of how all the "options" stuff work but I think it's a good
start for more refactorings (clearing up the interfaces) to happen
later.
With this, pretty-text and app/lib/text are mostly a kitchen sink
of loosely related text processing utilities.
After the refactor, a lot more code related to setting up the
engine are now loaded lazily, which should be a pretty nice win. I
also noticed that we are currently pulling in the `xss` library at
initial load to power the "sanitize" stuff, but I suspect with a
similar refactoring effort those usages can be removed too. (See
also #23790).
This PR does not attempt to fix the sanitize issue, but I think it
sets things up on the right trajectory for that to happen later.
Co-authored-by: David Taylor <david@taylorhq.com>
We have identified some third-party analytics scripts which do things like `window.I18n = window.I18n` 🤦♂️. This leads to the window object having a null I18n property, but `"I18n" in globalThis` returns true.
This commit checks whether `window.I18n` is a truthy value.
Nowadays, themes/plugins have their templates compiled at build-time, so there is no need for us to carry the template compiler on the frontend during tests
The motivation of this PR is to remove our dependence on Ember's 'named outlets', which are removed in Ember 4+.
At a high-level, the changes can be summarized as:
- The top-level `discovery` route is totally emptied of all logic. The HTML structure of the template is moved into the `<Discovery::Layout />` component for use by child routes.
- `AbstractTopicRoute` and `AbstractCategoryRoute` routes now both lean on the `DiscoverySortableController` and associated template. This controller is where most of the logic from the old top-level `discovery` controller has ended up.
- All navigation controllers/templates have been replaced with components. `navigation/categories`, `navigation/category` and `navigation/default` were very similar, and so they've all been combined into `<Navigation::Default>`. `navigation/filter` gets its own component.
- The `discovery/topics` controller/template have been moved into a new `<Discovery::Topics>` component.
Various other parts of the app have been tweaked to support these changes, but I've tried to keep that to a minimum.
Anything from `<TopicList>` down is untouched, which should hopefully mean that a large proportion of topic-list-customizing themes are unaffected.
For more information, see https://meta.discourse.org/t/282816
When submitting files through the form template upload field, we were having an issue where, although a validation error message was being presented to the user, the upload was still coming through, because `PickFilesButton`'s validation happens **after** the Uppy mixin finished the upload and hit `uploadDone`.
This PR adds a new overridable method to the Uppy mixin and overrides it with the custom validation, which now happens before the file is sent.
Additionally, we're now also using `uploadingOrProcessing` as the source of truth to show the upload/uploading label, which seems more reliable.
This work includes the following updates for the new lightbox:
- Show carousel by default if there are more than 1 image in a post
- Removes toggling of carousel on mobile - now always open if there are more than 1 image
- Updates swipe down gesture on mobile to close lightbox (previously used to toggle carousel)
- Removes swipe up gesture on mobile (was previously used to close lightbox)
This change removes the background image (which is the small version of the uploaded image) from the lightbox backdrop.
Now a solid color (dark grey) is used for the backdrop so we can distinguish between the lightbox's head, body and footer.
This aims to help admins and developers identify the cause of loading issues on routes.
As with other theme/plugin errors, the UI banner is only shown to administrators. For non-admins, the information is only written to the browser console.
When uploading images, they are assigned a dominant color which gets used in various places, such as Discourse Hub and the new lightbox. Previously in chat we didn't assign this attribute, so it was defaulting to a null value. We did however use it as an inline CSS style for the image background (which is visible while the image is downloaded).
This change adds data-dominant-color to the uploaded image in chat and uses it correctly within lightbox.
Followup to b53449eac9, we cannot
generate the links to plugin admin pages in this way because it
depends on which plugins are installed; we would need to somehow
do it at runtime. Leaving it out for now, for people who need to
find these admin routes the Ember Inspector extension for Chrome
can be used in the meantime.
Since we don't have icons or access to the JS that transforms
hashtag icon placeholders into their proper icons and colours
on embed and publish pages, we need to at least show _something_
and make sure the hashtags are not totally broken on these pages.
NOTE: Most of this is experimental and will be removed at a later
time, which is why things like translations have not been added.
The new /admin-revamp UI uses a sidebar for admin nav. This initial
step adds a script to generate a map of all the current admin nav
into a format the sidebar to read. Then, people can experiment
with different changes to this structure.
The structure can then be edited from `/admin-revamp/config/sidebar-experiment`,
and it is saved to local storage so people can visually experiment with different ways
of showing the admin sidebar links.
Two changes were introduced:
1. Reorder links on sidebar section is removed. Clicking and holding the mouse for 250ms was unintuitive;
2. Fixed bugs when reorder is done in edit modal.
This fixes an edge case where the layout of a onebox with a gif avatar
was broken. Oneboxes have specific styling attached to avatar images and
the pausable animated image treatment was breaking that styling.
- Add prefixes to Ember deprecations (previously was just Discourse deprecations)
- Allow logic to work in tests (where window.Discourse is not defined)
- Detect `{plugin}_tests.js` files
- Optimise dev/test regex logic out of the production build using `if(DEBUG)`
* UX: add discourseLater call to add breathing room for animation
Allow for smoother animations on lower end devices.
Create time between render and animations.
extend panel width targets by 20 px to account for shadows as well
This API is not used by any known themes/plugins, and is problematic for a few reasons
- It doesn't work on modern plugin connectors which have no wrapper element
- Making modifications to Ember-rendered DOM elements can lead to catastrophic and surprising errors
- It doesn't re-run when arguments to a plugin outlet change
This commit adds the deprecation notice, and refactors the tests so that they do not rely on any real core plugin outlets
plugin/theme-breaking changes:
1. `controller:create-account` is gone (use `component:modal/create-account` in modifyClass, **if** absolutely necessary)
2. `create-account-body` css class is gone (target `.d-modal.create-account` or any of the inner classes: `.modal-outer-container`, `.modal-middle-container`, `.modal-inner-container`, or `.modal-body`)
This commit fixes an issue where clicking the default
"Take Action" option on a flag for a post doesn't always
end up with the post hidden.
This is because the "take_action" score bonus doesn’t take into account
the final score required to hide the post.
Especially with the `hide_post_sensitivity` site setting set to `low`
sensitivity, there is a likelihood the score needed to hide the post
won’t be reached.
Now, the default "Take Action" button has been changed to "Hide Post"
to reflect what is actually happening and the description has been
improved, and if "Take Action" is clicked we _always_ hide the post
regardless of score and sensitivity settings. This way the action reflects
expectations of the user.
* FEATURE: Add keywords support for site_settings search
This change allows for a new `keywords` field that can be added to site
settings in order to help with searching. Keywords are not visible in
the UI, but site settings matching one of the contained keywords will
appear when searching for that keyword.
Keywords can be added for site settings inside of the
`config/locales/server.en.yml` file under the new `keywords` key.
```
site_settings
example_1: "fancy description"
example_2: "another description"
keywords:
example_1: "capybara"
```
* Add keywords entry for a recently changed site setting and add system specs
* Use page.visit now that we have our own visit
Some browsers still don't support conditional mediation. This PR fixes issues with:
- TOR browser (it doesn't have `PublicKeyCredential` at all)
- Firefox 119 (doesn't support conditional mediation)
We also need to make sure not to call `isConditionalMediationAvailable` on browsers that don't support the method but support the feature (like Safari on iOS).
This was just a case of removing the `onlyStream: true`
operation from `decorateCookedElement`, since that restricts
the decoration only to topic page posts.
This regressed in b6dc929. A test to ensure this doesn't regress has
been added as well.
This PR also fixes a flakey system spec. The conditional UI gets
triggered automatically, so the system spec shouldn't explicitly call
`find(".passkey-login-button").click`, because sometimes it isn't
present and that causes a test failure.
- Remove the wildcard crawler. This was already excluding almost all file types, but the exclude list was missing '.gjs' which meant those files were unnecessarily being hoisted into the `public/` directory during precompile
- Automatically include all ember-cli-generated assets without needing them to be listed. The main motivation for this change is to allow us to start using async imports via Embroider/Webpack. The filenames for those new async bundles will not be known in advance.
- Skips sprockets fingerprinting on Embroider/Webpack chunk JS files. Their filenames already include a fingerprint, and having sprockets change the filenames will cause problems for the async import feature (where filenames are included deep inside js bundles)
This commit also updates our ember-cli build so that it skips building plugin tests in the production environment. This should provide a slight build speed improvement.
Previously, focus wasn't being applied correctly on dialogs using named
components. This was because the A11YDialog was being invoked before
the component was completely rendered.
The long-term plan is to move away from A11YDialog doing the rendering
here, but for now this should do.
These have been deprecated for some time, and the vast majority of themes/plugins have already removed their use. The prototype extensions were unexpectedly disabled as a side effect of 895036bd7a (more details in https://github.com/discourse/discourse/pull/24101).
Given that restoring the functionality now involves significant complexity, and would only be delaying the inevitable removal in a matter of months, we've decided to keep them disabled. This commit explicitly sets the flag in the ember environment config to make things clearer.
We already do this check inside `selectionChanged` and this was preventing us to correctly set `isSelecting` to true. This was causing issues when starting your selection from outside cooked.
Applies to passkeys, visible in a dev environment when using a non-standard
host. The error modal should only be shown when invoking the passkey
login button.
The main change made is to use `pointerdown` and `touchstart` for detecting click outside in `FloatKit`, the problem of using `click` is that it will trigger on `mouseup` which is not working well with `FloatKit` shown using `mousedown` (when we change selection with the `mousedown` for example) as the release will be interpreted as a click outside and close the menu. To solve this issue the previous code in `post-text-selection` was going through various hacks for detecting state of mouse which are not always very reliable.
The second fix is to exit earlier when selection didn't change.
This has been tested on chrome/firefox and safari (mobile) and seems to work reliably.
<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
`@ember/jquery` was necessary to automate the `app.import()` but
that is no longer necessary with `ember-auto-import`. A secondary
thing it does is bringing back the `this.$` feature, but with a
deprecation. It is my understanding that the deprecation has long
be fully absorbed into both core and plugins so we shouldn't need
this package anymore.
No plugins or themes rely on anonymous_posting_min_trust_level so we
can just switch straight over to anonymous_posting_allowed_groups
This also adds an AUTO_GROUPS const which can be imported in JS
tests which is analogous to the one defined in group.rb. This can be used
to set the current user's groups where JS tests call for checking these groups
against site settings.
Finally a AtLeastOneGroupValidator validator is added for group_list site
settings which ensures that at least one group is always selected, since if
you want to allow all users to use a feature in this way you can just use
the everyone group.
Back in c31772879b we introduced
SiteSetting.composer_ios_media_optimisation_image_enabled and
disabled media optimization on safari iOS because of performance
issues when rendering to canvas, and OffscreenCanvas support
was not yet available.
Safari now supports OffscreenCanvas, so now we can give this
another go, and also use OffscreenCanvas everywhere it is supported.
This fixes a similar issue to 8b3eca0 where an Errno::ETXTBSY error was raised because the minio_runner gem was trying to install the binary across multiple processes in rspec. If we just make sure the latest version is installed before the tests run, this shouldn't happen, since MinioRunner.start will not do any further attempts at installation if the latest version is installed.
Running addonPostprocessTree manually was causing ember-auto-import's postprocess hook to run and generate extra unnecessary chunks. The only reason called addonPostprocessTree directly was to allow the terser plugin to run on the extra public trees. We can do the terser postprocessing manually instead.
This commit is approximately the inverse of e1d27400f5.
This commit also removes ember-auto-import as dependencies of admin/wizard/discourse-plugins because they are not 'real' ember addons, and so it isn't serving any useful purpose. (see also https://github.com/discourse/discourse/pull/23974)
As much as possible I would like us to avoid having to go the with a global event listener on click/mouseover. For now I have removed all cases of `data-tooltip`, if we clearly identify a use case of a global event listener we might reconsider this.
The following changes are also included:
- by default tooltips won't attempt to focus first focusable element anymore
- tooltip will now use `cursor: pointer` by default
- a new service has been introduced: `InternalTooltip` which is responsible to track the current instance displayed by a `<DTooltip />`. Portal elements when replaced are not properly cleaned and I couldn't figure out a way to have a proper hook to ensure the previous `DTooltipInstance` is properly set as not expanded; this problem was very visible when using a tooltip as interactive and hovering another tooltip, which would replace the interactive tooltip as not closed.
This allows users to see their passkeys recommended by the browser as they type their username.
There's a small refactor here, to make sure the same action is used by both the conditional UI and the passkey login button. The webauthn API only supports one auth attempt at a time, so in this PR we need to add a service singleton to manage the navigator.credentials.get promise so that it can be cancelled and reused as the user picks the conditional UI (i.e. the username login input) or the dedicated passkey login button.
This commit adds a loading spinner that appears immediately after
clicking the play button on a video placeholder and will go away once
the "onCanPlay" event fires for the video.
This prevents a completely empty (no play button) placeholder from
appearing for some amount of time while the video is loading enough to
start playing.
- more subtle animation when showing a toast
- resumes auto close when removing the mouse from the toast
- correctly follows reduced motion
- uses output with role status as element: https://web.dev/articles/building/a-toast-component
- shows toasts inside a section element
- prevents toast to all have the same width
- fixes a bug on mobile where we would limit the width and the close button wouldn't show correctly aligned
I would prefer to have tests for this, but the conjunction of css/animations and our helper changing `discourseLater` to 0 in tests is making it quite challenging for a rather low value. We have system specs using toasts ensuring they show when they should.
Why this change?
When we're in the midst of loading more tags, the filter dropdown
is still enabled and may result in us firing off multiple requests to
the server to load more tags. This makes the loading hard to reason
about in the tests environment and has led to flaky tests.
What does this change do?
This changes disables the filter dropdown when more tags are being
loading.
This commit fixes a bug in which the dark category logo would be used in a light theme if the system preference was set to dark and the user forced the use of a light theme in Discourse
This PR addresses the push to unify the icon representing AI throughout Discourse, by using the discourse-sparkles icon.
The icon is being moved to core to make changes with dependencies included in core that were using the "magic" icon instead.
In 2 places "magic" -> "discourse-sparkles,
1. topic summaries
2. (unreleased) chat summaries example
This widget is no longer used. It's better to remove it completely, so that `decorateWidget` and `reopenWidget` calls print a warning to the console rather than failing silently.
* FIX: Don't lose SummaryBox state through widget re-renders.
The <SummaryBox /> component state will get lost when scrolling to the bottom of a topic. Due to the widget being re-rendered, it will go back to the collapsed state, and we need to fetch the summary again.
This change moves all the state updates to the postStream model, which also refreshes the widget to keep it updated.
* Reify topic summary using a pojo
Now that core has a file structure and default imports, Ember's resolver can load helpers lazily. So we can remove the lazy loading, and helpers in ember templates will continue to work. This should provide a slight performance improvement for initial boot.
However, there is a slight complication: some of our helpers are also registered with our Raw Handlebars system as a side-effect of loading the module. Therefore, this commit adds a `helperMissing` helper to our RawHandlebars system. This looks up the helper by name in the ember resolver, which triggers the relevant module to be evaluated, and the raw helper to be registered as a side effect.
For backwards-compatibility, plugin and theme helpers continue to be eagerly evaluated. Once the `discourse.register-unbound` deprecation is resolved, we can safely remove this eager loading.
`registerUnbound` was present for legacy reasons when using helpers in raw-hbs and has been replaced by `registerRawHelper`.
For new helpers used only in classic ember template, exporting a default function from `helpers/*.js` is recommended.
This change also means that all existing helpers will be available to import in `gjs` files.
Co-authored-by: David Taylor <david@taylorhq.com>
This commit adds a new admin UI under the route `/admin-revamp`, which is
only accessible if the user is in a group defined by the new `enable_experimental_admin_ui_groups` site setting. It
also adds a special `admin` sidebar panel that is shown instead of the `main`
forum one when the admin is in this area.
We also add an "Admin Revamp" sidebar link to the community section, which
will only appear if the user is in the setting group:
Within this there are subroutes defined like `/admin-revamp/config/:area`,
these areas could contain any UI imaginable, this is just laying down an
initial idea of the structure and how the sidebar will work. Sidebar links are
currently hardcoded.
Some other changes:
* Changed the `main` and `chat` panels sidebar panel keys to use exported const values for reuse
* Allowed custom sidebar sections to hide their headers with the `hideSectionHeader` option
* Add a `groupSettingArray` setting on `this.siteSettings` in JS, which accepts a group site setting name
and splits it by `|` then converts the items in the array to integers, similar to the `_map` magic for ruby
group site settings
* Adds a `hidden` option for sidebar panels which prevents them from showing in separated mode and prevents
the switch button from being shown
---------
Co-authored-by: Krzysztof Kotlarek <kotlarek.krzysztof@gmail.com>
* UX: add static confetti bacgkround image on wizard steps
* DEV: slow down speed animation for confetti
* DEV: compress image file size
* UX: use an image that has transparent background
* DEV: use correct image file name
- don't try to guess the name of the manager (too many options)
- improve error message when registration is not allowed
- output error in console when registration fails
- minor fix to rename dialog layout
- hides action buttons in DiscourseHub (because adding passkeys there is not possible)
- adds acceptance test to ensure action buttons are hidden for admins seeing another user's profile
This API came from a time when themes had to define JS and templates inside `<script>` tags. Nowadays, it's rarely used, and much better patterns are available for registering connectors.
These updates significantly improve IDE tooling for imports across the Discourse core codebase, and also for framework packages. The `@types/ember-*` packages are a temporary solution until we get onto Ember 5, which ships its types in the main package.
The previous approach of having jsconfig files in each package directory did work, but once you start adding all the possible interlinks between them, we hit the file count limit of VSCode's tooling (because it counts every file for every jsconfig its referenced in). Having one file at the root means that a single file can apply to all core packages and plugins.
Long-term, to get the same functionality for all themes/plugins, we may need to look at building/publishing a Discourse types package which can be added to theme/plugin package.json files for development purposes.
As of #23867 this is now a real package, so updating the imports to
use the real package name, rather than relying on the alias. The
name change in the package name is because `I18n` is not a valid
name as NPM packages must be all lowercase.
This commit also introduces an eslint rule to prevent importing from
the old I18n path.
For themes/plugins, the old 'i18n' name remains functional.
Why this change?
In 38d3208027, the position of the
`headerBelowTitle` outlet was changed causing the deselect text in the
edit sidebar catgegory/tag modals to appear inline with the title which
we do not want.
What does this change do?
This change introduces the `belowModalTitle` outlet in `DModal` which is
where the `headerBelowTitle` outlet was located before it was changed.
This reverts commit 5f0bc4557f.
Through extensive internal discussion we have decided to revert
this change, as it significantly impacted moderation flow for
some Discourse site moderators, especially around "something else"
flags. We need to re-approach how flags are counted holistically,
so to that end this change is being reverted.
This commit introduces a new endpoint to search categories and uses it
instead of the categories map that is preloaded using SiteSerializer.
This feature is enabled only when the hidden site setting
lazy_load_categories is enabled and should be used only on sites with
many categories.
At this moment, this feature is under a site setting named
lazy_load_categories.
In the future, categories will no longer be preloaded through site data.
This commit add information about categories in topic list and ensures
that data is used to display topic list items.
Parent categories are serialized too because they are necessary to
render {{category-link}}.
We'll probably have to keep the globals around for compatibility, but we should always import it ourselves. We'll followup with an updated eslint config to enforce this.
PERF: improve touch, swipe, panning performance on mobile menus
---
* stop event propagation on swipe events: other touch events were stealing a huge amount of time here. Stop event
propagation when handling pan events.
* animate with [web animations api](https://developer.mozilla.org/en-US/docs/Web/API/Web_Animations_API/Using_the_Web_Animations_API)
* prefer translate3d to hint for gpu rendering.
* query document for elements only on start move event, not on subsequent move
events
* remove unused calculations for directioned velocity and distance: all swipe/pan elements function in x/y direction only.
* re-implement scroll locking behavior.
re-implemented scroll lock behavior
---
With stop event propagation, we need to re-implement scroll locking on menu swipes.
Previously, this was using onTouchMove which was costly.
We may now use styling with overflow-y:hidden to lock scroll behavior.
overflow:hidden on html/body elements is now supported by iOS as of 2022
https://bugs.webkit.org/show_bug.cgi?id=153852https://bugs.webkit.org/show_bug.cgi?id=220908
UX: improve swipe
---
Some improvements to get gestures and swipes feeling a little more polished.
This focuses on end gesture, and how we transfer it to a css animation to
complete a menu open/close action.
Multitouch: events may pan, scroll, and zoom - especially on iOS safari.
Cancelling the swipe event allows for a more pleasant zooming experience.
* ease-out on menus opening, linear on close
* calculate animation duration for opening and closing,
attempt to better transfer user swipe velocity to css animation.
* more timely close/open and cleanup from calculated animation timing.
* add animation to closing menus on cloak tap
* correctly animate menus with ease-in and ease-out
* add swipe cancel event on multitouch event
DEV
---
* lean on promises
js animations api gives us promises to listen to. Update test waiters
to use waitForPromise from @ember/test-waiters instead of reigster/unregister.
* convert swipe mixin to its own class.
Convert swipe callbacks to custom events on the element.
Move shared functions for max animation time and close logic to
new shared class.
swipe-events lib uses custom events to trigger callbacks, rather than assuming
implemented hard coded function from the mixin's base class. Custom events are
triggered from the bound element as swipestart, swipeend, swipe
Add shared convenience functions for swipe events so they can be more easily
shared.
A client receives an initial swipe event and can check some state to see if it
wants to handle the swipe event and if it doesn't, calling
`event.preventDefault();` will prevent `swipe` and `swipeend` events from firing
until another distinct swipestart event is fired. Swipe events will auto-cancel on multitouch.
The scroll lock has also exposed as its own utility class.
Adds UI elements for registering a passkey and logging in with it. The feature is still in an early stage, interested parties that want to try it can use the `experimental_passkeys` site setting (via Rails console).
See PR for more details.
---------
Co-authored-by: Joffrey JAFFEUX <j.jaffeux@gmail.com>
Previously this logic was only checking the post number. That meant that navigating between the first post of two topics would not trigger the event.
In the past, the event would be triggered anyway because the ScrollingPostStream would be destroyed/re-created when navigating between topics. But now that we use the 'loading slider' technique, the same component instance is re-used.
The motivation for this commit is to fix the 'DiscoToc' theme component, which relies on the event firing when navigating between topics.
This commit adds a new Revise... action that can be taken
for queued post reviewables. This will open a modal where
the user can select a Reason from a preconfigured list
(or by choosing Other..., a custom reason) and provide feedback
to the user about their post.
The post will be rejected still, but a PM will also be sent to
the user so they have an opportunity to improve their post when
they resubmit it.
Preloading just metadata is not always respected by browsers, and
sometimes the whole video will be downloaded. This switches to using a
placeholder image for the video and only loads the video when the play
button is clicked.
Currently, `window.I18n` is defined in an old school hand written
script, inlined into locale/*.js by the Rails asset pipeline, and
then the global variable is shimmed into a pseudo AMD module later
in `module-shims.js`.
This approach has some problems – for one thing, when we add a new
V2 addon (e.g. in #23859), Embroider/Webpack is stricter about its
dependencies and won't let you `import from "I18n";` when `"I18n"`
isn't listed as one of its `dependencies` or `peerDependencies`.
This moves `I18n` into a real package – `discourse-i18n`. (I was
originally planning to keep the `I18n` name since it's a private
package anyway, but NPM packages are supposed to have lower case
names and that may cause problems with other tools.)
This package defines and exports a regular class, but also defines
the default global instance for backwards compatibility. We should
use the exported class in tests to make one-off instances without
mutating the global instance and having to clean it up after the
test run. However, I did not attempt that refactor in this PR.
Since `discourse-i18n` is now included by the app, the locale
scripts needs to be loaded after the app chunks. Since no "real"
work happens until later on when we kick things off in the boot
script, the order in which the script tags appear shouldn't be a
problem. Alternatively, we can rework the locale bundles to be more
lazy like everything else, and require/import them into the app.
I avoided renaming the imports in this commit since that would be
quite noisy and drowns out the actual changes here. Instead, I used
a Webpack alias to redirect the current `"I18n"` import to the new
package for the time being. In a separate commit later on, I'll
rename all the imports in oneshot and remove the alias. As always,
plugins and the legacy bundles (admin/wizard) still relies on the
runtime AMD shims regardless.
For the most part, I avoided refactoring the actual I18n code too
much other than making it a class, and some light stuff like `var`
into `let`.
However, now that it is in a reasonable format to work with (no
longer inside the global script context!) it may also be a good
opportunity to refactor and make clear what is intended to be
public API vs internal implementation details.
Speaking of, I took the librety to make `PLACEHOLDER`, `SEPARATOR`
and `I18nMissingInterpolationArgument` actual constants since it
seemed pretty clear to me those were just previously stashed on to
the `I18n` global to avoid polluting the global namespace, rather
than something we expect the consumers to set/replace.
We run the ember-this-fallback transformation on plugin and theme code so that they can continue omitting `this.` in `.hbs` templates. A bug in the implementation meant that it was incorrectly transforming things like `{{dir/some-component}}` into `<DirSomeComponent />` (rather than `<Dir::SomeComponent />`).
This commit uses patch-package to apply the fix from https://github.com/tildeio/ember-this-fallback/pull/56
`escape` from `pretty-text/sanitizer` is a re-export of the same
function defined in `discourse-common`. Updating the import paths
across the codebase to use the `discourse-common` import path.
`escape` is a rather simple function that can be accomplished with
a regular expression in `discourse-common`.
On the other hand, the remaining parts in `pretty-text/sanitizer`
has a lot of code, PLUS it depend on the rather heavy "xss" NPM
library.
Currently, most of the consumers of `pretty-text/sanitizer` are of
the `{ escape }` varient. This is resolved by this PR.
The remaining usages are either:
1. via/through `PrettyText` which is essentially gated behind
loading the markdown-it bundle, OR
2. via `sanitize` from `discourse/lib/text`
I believe we may ultimately be able to move all the usages to behind
the markdown-it bundle (or, equivilantly, set up another lazy bundle
for `sanitize`) and be able to shed the sanitization code and the
"xss" library from the initial page load.
`discourse/lib/text` also defines a `sanitizeAsync` which is gated
behind loading the markdown-it bundle.
Looking through the usages of `sanitize`, I believe most of these
can be safely switched to use `sanitizeAsync`, in that they are
already in an asynchrnous path that handles a server response. Most
of them are actually rendering a piece of server-generated HTML
message as flash message, so I am not sure there really is value in
sanitizing (we should be able to trust our own server?), but in any
case, code-wise, they should already be able to absorb the async
just fine.
I am not sure if `sanitize` and `sanitizeAsync` are actually API
compatible – they both take `options` but I think those `options` do
pretty different things. This is somethign for another person to
investigate down the road in another PR.
According to `all-the-plugins`, `discourse-graphviz` also import
from this location, so perhaps we should PR to update. That being
said, it doesn't really hurt anything to keep the alias around for
a while.
This started out as a seemingly benign refactor to replace the
`require` for `withPluginApi` to an actual import. However, it
broke the test in seemingly random places.
It turns out that in serveral places, we are calling `isTesting()`
in module scope and assigning the result to a constant. For example
we do that in the composer service to disable checking drafts when
testing.
This is problematic because `isTesting` doesn't really set until
the `discourse-bootstrap` initializer is run, and so any modules
that are evaluated before then will have locked in the wrong value
for `isTesting()`.
If we are going to use and treat `isTesting()` like a constant then
we will have to make sure we set it sufficiently early before any
code-loading happens.
Previously, the `user-tips` service included a couple of calls to `next()`. These were introduced to work around errors like
```
You attempted to update `availableTips` on `<UserTips:ember659>`, but it had already been used previously in the same computation
```
These errors come from the fact that various `<UserTip>` components are rendering at slightly different times in the runloop and stepping on each other. Normally this doesn't happen in Ember, but the implementation details of our 'Widget' system and its 'RenderGlimmer' helper mean that RenderGlimmer components are rendered later than normal Ember components. Using `next()` avoids the problem because it means that all the updates are scheduled together in the following runloop interation.
However, the use of `next()` can create some subtle timing issues, which have been evident in the recent flakiness of some qunit tests. This commit makes a few changes to improve the situation:
1. Use a TrackedMap to provide fine-grained `shouldRender()` reactivity for each user-tip id. That means that different user tips will not be trying to update the same piece of tracked state (previously the entire `availableTips` array was `@tracked`, and was completely re-assigned every time a new `<UserTip>` was rendered
2. Avoid reassigning any tracked state unless the value has actually changed
3. Remove the `next()` workarounds
- Introduces a `deepFreeze` helper to block any mutations to the current-user fixture
- Add `cloneJSON` to any places which were previously causing mutations
Currently, the UI section that contains the title+category+tags of a topic list item (the mobile version) has only one and very generic CSS class, `.right`. Plugins and themes that need to target this section for styling would have to use awkward/very specific CSS selectors in order to avoid incorrectly styling other elements that happen to have the same generic CSS class.
This commit adds an additional class `.topic-item-metadata` to the section to allow easier and more maintainable styling for it.
See https://github.com/discourse/discourse-clickable-topic/pull/4 for a theme that will benefit from this change.
Normally, modules defined under `blah/index` can be imported as `blah`. This is also true of Ember resolver lookups - `<MyComponent />` should resolve to the same as `<MyComponent::Index />`. This was working as expected in Discourse core, but we had not implemented the same in our custom resolver logic for themes/plugins.
This commit implements the `/index` fallback, and adds a test for the behaviour.
The 'create topic' entry in the dropdown was incorrectly using the 'reply as new topic' description. This fixes the logic to use a separate locale key for the description.
This commit does a couple of things:
1. Add a new plugin outlet, `above-topic-list-item`, to the `topic-list-item` component
2. Pass the topic in question as an outlet argument for the (existing) `above-latest-topic-list-item` outlet in the `latest-topic-list-item` component.
For the admin plugin list we want to be able to link to
a meta topic for plugins, but we have no standard way to
do this at the moment. This adds support for meta_topic_id
alongside other plugin metadata like authors, URL etc,
that gets built into a Meta topic URL in the serializer.
Some time ago, we introduced the `cookAsync` instead of the existing
`cook` function, and planned to migrate everything to it. Then after
migrating, we wanted to raname the function to simply `cook`.
I've checked Core and plugins, and currently we call `cookAsync` everywhere,
there are no calls to the `cook` function anymore. So we're good
to proceed with this refactoring.
This PR makes the first step by making current cookAsync and cook functions
do the same thing. Effectively now the `cook` function becomes an alias
for the `cookAsync` function.
This PR is a first step towards private groups. It redesigns settings/members area of a channel and also drops the "about" page which is now mixed into settings.
This commit is also:
- introducing chat-form, a small DSL to create forms, ideally I would want something in core for this
- introducing a DToggleSwitch page object component to simplify testing toggles
- migrating various components to gjs
Why this change?
Back in May 17 2023 along with the release of Discourse 3.1, we announced
on meta that the legacy hamburger dropdown navigation menu is
deprecated and will be dropped in Discourse 3.2. This is the link to the announcement
on meta: https://meta.discourse.org/t/removing-the-legacy-hamburger-navigation-menu-option/265274
## What does this change do?
This change removes the `legacy` option from the `navigation_menu` site
setting and migrates existing sites on the `legacy` option to the
`header dropdown` option.
All references to the `legacy` option in code and tests have been
removed as well.
Currently, if you set an integer site setting in the admin interface and include thousands separators, you will silently configure the wrong value.
This PR replaces TextField inputs for integer site settings with NumberField. It also cleans the numeric input of any non-digits in the backend in case any separators make it through.
The custom html elements we were using for bootstraping were causing Embroider to end the `<head>` tag and immediately start `<body>`. As a result most of `<meta>` tags ended up in the `<body>`.
That mean (among possibly other issues) that the app did not have CSRF token set properly on launch (in the development env)
Why this change?
Previously just using the `addToolbarPopupMenuOptionsCallback` plugin
API itself was insufficient because it required the return object to
include an `action` key which only accepted a name of the action
function as a string. This was highly problematic because the action
function had to be defined on the `composer` service which means using
the `modifyClass` API to add the action function. This made the API
awkward to use leading to poor developer experiencec.
What does this change do?
This commit introduces a couple of improvemnts to the API.
1. First the API has been renamed to `addComposerToolbarPopupMenuOption` because
the API no longer accepts a callback function which was quite
redundant. Instead, it now accepts an Object. The
`addToolbarPopupMenuOptionsCallback` API function is deprecated and
will be dropped in Discourse 3.3. Note that passing the API a
function is still supported but will be dropped when the `addToolbarPopupMenuOptionsCallback`
is removed.
2. The `action` key in the Object passed to the function can now be a
function and is passed the `toolbarEvent` object when called.
3. The `condition` on key in the Object passed to the function can now be a
function and is passed the `composer` service when called.
It's a special case widget - its constructor has different contructor arguments:
```js
export default class PostCooked {
constructor(attrs, decoratorHelper, currentUser) {
...
```
vs
```js
export default class Widget {
constructor(attrs, register, opts) {
...
```
Until now, plugins/themes had to follow very specific directory structures to set up plugin outlet connectors. This commit introduces a new `api.renderInOutlet` API which makes things much more flexible. Any Ember component definition can be passed to this API, and will then be rendered into the named outlet.
For example:
```javascript
import MyComponent from "discourse/plugins/my-plugin/components/my-component";
api.renderInOutlet('user-profile-primary', MyComponent);
```
When using this API alongside the gjs file format, components can be defined inline like
```javascript
api.renderInOutlet('user-profile-primary', <template>Hello world</template>);
```
This commit brings two fixes.
- increase the delay to trigger the action menu
- check of user activation before using vibrate:
https://developer.mozilla.org/en-US/docs/Glossary/Sticky_activationhttps://developer.mozilla.org/en-US/docs/Web/Security/User_activationhttps://developer.mozilla.org/en-US/docs/Web/API/UserActivation/hasBeenActive
> Sticky activation is a window state that indicates a user has pressed a button, moved a mouse, used a menu, or performed some other user interaction. It is not reset after it has been set initially (unlike transient activation).
> APIs that require sticky activation (not exhaustive):
> - Navigator.vibrate()
> - VirtualKeyboard.show()
> - Autoplay of Media and Web Audio APIs (in particular for AudioContexts).
Before this fix, we could end up with this error in the console in tests:
> Blocked call to navigator.vibrate because user hasn't tapped on the frame or any embedded
<!-- NOTE: All pull requests should have tests (rspec in Ruby, qunit in JavaScript). If your code does not include test coverage, please include an explanation of why it was omitted. -->
Regression from https://github.com/discourse/discourse/pull/23668 where we stopped passing in `this.badgeReason` to the badge granting function. This PR fixes that and adds a unit test to cover that code path.
Ember expects a trailing slash on this value, which is different to the Rails app's behavior. Values without a trailing slash seemed to work for legacy ember-cli builds, but would lead to errors under embroider.
This PR converts the post notice modal from the old template + controller to a modern Glimmer + DModal component.
In addition to the conversion, I added a condition so that when editing a staff notice, the save button is disabled as long as no changes have been made.
This reverts commit 42070d49da.
Overriding Error.stack like this seems to break the browser's own sourcemapping of stack-traces. Plus, it adds quite a significant performance overhead to tests (QUnit seems to rely on Error.stack even when tests pass). Reverting for now, but perhaps we can build a way to make this only apply to the UI-displayed stack traces in future 🤔
When navigating between renewables through the Ember router, e.g. through the links in the notifications menu the body of the reviewable (rendered by the CookText component) won't update, resulting in the same post body incorrectly being shown for all subsequent reviewables.
This is happening because there is no update path between the rawText attribute being passed to CookText and the computed cooked attribute, since this is being set explicitly using an async function.
This PR adds the missing link between rawText and cooked by listening for didUpdate and triggering the async function.
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.
