Passing through `attrs` is problematic for a few reasons:
1. Connectors could mutate it and cause issues in the parent widget
2. It doesn't provide a clean API boundary. The connector can access all attrs of the widget. As we move towards refactoring the header away from widgets, this may change. Better to explicitly call out the things we expect plugins/themes to access
3. `attrs` is a reserved property for classic components. Passing an argument called `attrs` into a classic component raises a 'computed property override' deprecation error under Ember 3.28, and causes an error in Ember 4+.
Unfortunately this will be a breaking change to the outlet. Fortunately, it was introduced fairly recently and does not have too many users. We will make immediate updates to themes/plugins we are aware of.
Followup to 9cc2b5cc20
Commit dcd81d56c0 changed this, but that
implementation is not ideal because the initialization of the select kit
can result in requests to the server.
This implementation has the advantage that it also fixes the user and
group properties that return categories.
A lot of work has been put in the select kits used for selecting
categories: CategorySelector, CategoryChooser, CategoryDrop, however
they still do not work as expected when these selectors already have
values set, because the category were still looked up in the list of
categories stored on the client-side Categrories.list().
This PR fixes that by looking up the categories when the selector is
initialized. This required altering the /categories/find.json endpoint
to accept a list of IDs that need to be looked up. The API is called
using Category.asyncFindByIds on the client-side.
CategorySelector was also updated to receive a list of category IDs as
attribute, instead of the list of categories, because the list of
categories may have not been loaded.
During this development, I noticed that SiteCategorySerializer did not
serializer all fields (such as permission and notification_level)
which are not a property of category, but a property of the relationship
between users and categories. To make this more efficient, the
preload_user_fields! method was implemented that can be used to
preload these attributes for a user and a list of categories.
This commit ports the feature by @chapoi that was
previously a theme component in core.
A new post_menu button, copyLink, is added and used
as the default instead of share.
copyLink, on desktop, will copy the link of the post
to the user's clipboard and show a nice 'lil animation.
On mobile the native share menu will be shown.
If site owners want the old behaviour back, they just
need to change the post_menu site setting to use
the share button instead of copyLink.
The `search-menu-results-top` plugin outlet was not positioned at the top of `results` as expected. Additionally, the expected outlet arguments that existed in the "widget implementation" were not available on the glimmer search menu.
This commit refactors the Wizard component code in preparation for moving it to the 'static' directory for Embroider route-splitting. It also includes a number of general improvements and simplifications.
Extracted from https://github.com/discourse/discourse/pull/23678
Co-authored-by: Godfrey Chan <godfreykfc@gmail.com>
Why this change?
The tags modal loads more tags via infinite loading based on when the last tag in the
given page appears in the viewport for the user. When it comes in to
view, a request is then triggered to fetch additional tags. To ensure
that we are only loading a single page of tags each time the modal is
opened, we previously set a max height on the modal's body to ensure
that the last tag which appears in the modal will be outside of the view
port in the initial load. However, this has regressed recently due to
unknown reasons and resulted in multiple pages of tags being loaded
immediately from the server as the modal's height was not restricted.
This regression was caught by an existing test but was unfortunately
determined as flaky.
What does this change do?
This change restores the max height on the edit navigation menu tags
modal on dekstop.
Introduces the concept of image thumbnails in chat, prior to this we uploaded and used full size chat images within channels and direct messages.
The following changes are covered:
- Post processing of image uploads to create the thumbnail within Chat::MessageProcessor
- Extract responsive image ratios into CookedProcessorMixin (used for creating upload variations)
- Add thumbnail to upload serializer from plugin.rb
- Convert chat upload template to glimmer component using .gjs format
- Use thumbnail image within chat upload component (stores full size img in orig-src data attribute)
- Old uploads which don't have thumbnails will fallback to full size images in channels/DMs
- Update Magnific lightbox to use full size image when clicked
- Update Glimmer lightbox to use full size image (enables zooming for chat images)
We were previously relying on Ember's 'vendor' bundle to make the jquery global available on the activate_account route. That no longer happens under our Ember 5 build.
This commit updates our activate-account script to remove the need for jquery, so that it works under both Ember 3 and Ember 5 builds.
Using `DiscourseURL.routeTo` with `replaceURL: true` wouldn't cause a true Ember redirect. That meant that the transition source would be be replaced in the browser's history stack, and the 'back' button wouldn't work as expected. Instead, we can use the router service to perform a proper redirect.
We funnel vendored javascript through ember-cli, but that's only used for the testem environment. Therefore, there's no need to minify it in production builds. In my tests, this reduces peak RSS of a production build from 3.53GB to 3.15GB.
Since 4425e99bf9, we no longer ship the template compiler to the client under any circumstances, so this shim doesn't work. Plus, even if it did work, it would trigger the ember-global deprecation and fail under Ember 4+.
Followup e37fb3042d
* Automatically remove the prefix `Discourse ` from all the plugin titles to avoid repetition
* Remove the :discourse_dev: icon from the author. Consider a "By Discourse" with no labels as official
* We add a `label` metadata to plugin.rb
* Only plugins made by us in `discourse` and `discourse-org` GitHub organizations will show these in the list
* Make the plugin author font size a little smaller
* Make the commit sha look like a link so it's more obvious it goes to the code
Also I added some validation and truncation for plugin metadata
parsing since currently you can put absolutely anything in there
and it will show on the plugin list.
These properties are set on the "Manage > Categories" group page. It
used to work, but only because it overridden the properties and it did
not update the IDs too.
The category drop was rerendered after every category async change
because it updated the categories list. This is not necessary and
categories can be referenced indirectly by ID instead.
This value is included when generating static asset URLs. Updating the value will allow site operators to invalidate all asset urls to recover from configuration issues which may have been cached by CDNs/browsers.
Admin can add tag description up to 1000 characters.
Full description is displayed on tag page, however on topic list it is truncated to 80 characters.
This commit introduces the scaffolding for us to easily switch between Ember 3.28 and Ember 5 on the `main` branch of Discourse. Unfortunately, there is no built-in system to apply this kind of flagging within yarn / ember-cli. There are projects like `ember-try` which are designed for running against multiple version of a dependency, but they do not allow us to 'lock' dependency/sub-dependency versions, and are therefore unsuitable for our use in production.
Instead, we will be maintaining two root `package.json` files, and two `yarn.lock` files. For ember-3, they remain as-is. For ember5, we use a yarn 'resolution' to override the version for ember-source across the entire yarn workspace.
To allow for easy switching with minimal diff against the repository, `package.json` and `yarn.lock` are symlinks which point to `package-ember3.json` and `yarn-ember3.lock` by default. To switch to Ember 5, we can run `script/switch ember version 5` to update the symlinks to point to `package-ember5.json` and `package-ember3.json` respectively. In production, and when using `bin/ember-cli` for development, the ember version can also be upgraded using the `EMBER_VERSION=5` environment variable.
When making changes to dependencies, these should be made against the default `ember3` versions, and then `script/regen_ember_5_lockfile` should be used to regenerate `yarn-ember5.lock` accordingly. A new 'Ember Version Lockfiles' GitHub workflow will automate this process on Dependabot PRs.
When running a local environment against Ember 5, the two symlink changes will show up as git diffs. To avoid us accidentally committing/pushing that change, another GitHub workflow is introduced which checks the default Ember version and raises an error if it is greater than v3.
Supporting two ember versions simultaneously obviously carries significant overhead, so our aim will be to get themes/plugins updated as quickly as possible, and then drop this flag.
- Update optional-features to tie the `jquery-integration` flag to the current ember version
- Wrap ember-4-specific logic in ember-cli-build with a version check
- Update global-compat.js to add the jquery global if it doesn't exist (i.e. if we're on a modern ember version)
Extracted from https://github.com/discourse/discourse/pull/21720. This is a no-op under our current Ember 3.28 version.
- Skip rendering DModalLegacy when running Ember 5
- Move named outlet inside the DModalLegacy component file
- Exclude that DModalLegacy template from the build when running Ember 5
- Skip LegacySupport version of modal service when running Ember 5
- Add error popup for legacy modals when running Ember 5
Extracted from https://github.com/discourse/discourse/pull/21720. This is a no-op under our current Ember 3.28 version.
In modern versions of Ember, `this.parentView` is called internally during component init. We don't want our deprecation message to be triggered by that internal call, so we need an additional check.
Extracted from https://github.com/discourse/discourse/pull/21720
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.
Recently, we disabled the option to reorder links directly from the sidebar. Instead, user has to go to edit modal.
https://github.com/discourse/discourse/pull/24188
However, move cursor was left, which is misleading.
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.
