The problem:
Removing the options to addEventListener results in events that are properly cleaned up when the search menu is removed. Previously every time you opened the search menu, the listeners would be attached again, and clicking outside even after it was closed would fire the function again and again (N times as you opened the search menu!)
This was made far far worse in this commit c91d053, where I called close() to remove focus from the search input in the event that the search menu is rendered outside the header.
The problem with this was 2-fold. The close function tried to focus the search header button in core here. When the events aren't cleanup up and that happens... you can't do anything in the app.
The solution:
We don't need the event listeners to close the search menu when it's rendered from the header. The widget header handles clicks outside of the header. Sooo
1. Only register them for standalone search menus
2. Remove the passive options to the listeners so that they are properly removed on close
3. Call close() to unfocus input rather than just closing panel
4. Rename passed in are closeSearchMenu -> onClose because it's more accurate. It's really a callback.
Why this change?
The `Editing sidebar tags navigation allows a user to filter the tag in the modal by selection` system test was flaky
when we were doing `modal.filter("").filter_by_unselected`. The
hypothesis here is that the filtering is debounced before issue a
request to load the new tags and the dropdown is only disabled in the
debounced function. Thereforethere is a chance that when
`modal.filter_by_unselected` runs, it is selecting a row against a
disabled dropdown which results in a noop.
What does this change do?
When filtering using the input in the modal, we will now disabled the
dropdown until the filtering completes which will then re-enable the
dropdown.
The `style` variable is always set because every category has a color
defined, so the surrounding if statement is unnecessary.
"+ X categories" option has also been removed in the past and the code
related to it is now dead code.
This changes the Plugins link in the admin sidebar to
be a section instead, which then shows all enabled plugin
admin routes (which are custom routes some plugins e.g.
chat define).
This is done via adding some special preloaded data for
all controllers based on AdminController, and also specifically
on Admin::PluginsController, to have the routes loaded without
additional requests on page load.
We just use a cog for all the route icons for now...we don't
have anything better.
We had our own implementation of number fields in Ember, extended from text fields. Number inputs are now widely supported in browsers, and we can fall back on the native implementation which will be a better experience in almost all cases.
One thing traded off here is number fields can't have a placeholder, but that is intentional. We aren't using that ability anywhere, and we probably only kept it because we're extending text fields.
With this change we can get rid of the entire .js file, since there's no custom behaviour, and just make NumberField a template.
(extracted from #23678)
* Move Wizard back into main app, remove Wizard addon
* Remove Wizard-related resolver or build hacks
* Install and enable `@embroider/router`
* Add "wizard" to `splitAtRoutes`
In a fully optimized Embroider app, route-based code splitting more
or less Just Work™ – install `@embroider/router`, subclass from it,
configure which routes you want to split and that's about it.
However, our app is not "fully optimized", by which I mean we are
not able to turn on all the `static*` flags.
In Embroider, "static" means "statically analyzable". Specifically
it means that all inter-dependencies between modules (files) are
explicitly expressed as `import`s, as opposed to `{{i18n ...}}`
magically means "look for the default export in app/helpers/i18n.js"
or something even more dynamic with the resolver.
Without turning on those flags, Embroider behaves conservatively,
slurps up all `app` files eagerly into the primary bundle/chunks.
So, while you _could_ turn on route-based code splitting, there
won't be much to split.
The commits leading up to this involves a bunch of refactors and
cleanups that 1) works perfectly fine in the classic build, 2) are
good and useful in their own right, but also 3) re-arranged things
such that most dependencies are now explicit.
With those in place, I was able to move all the wizard code into
the "app/static" folder. Embroider does not eagerly pull things from
this folder into any bundle, unless something explicitly "asks" for
them via `imports`. Conversely, things from this folder are not
registered with the resolver and are not added to the `loader.js`
registry.
In conjunction with route-based code splitting, we now have the
ability to split out islands of on-demand functionalities from the
main app bundle.
When you split a route in Embroider, it automatically creates a
bundle/entrypoint with the relevant routes/templates/controllers
matching that route prefix. Anything they import will be added to
the bundle as well, assuming they are not already in the main app
bundle, which is where the "app/static" folder comes into play.
The "app/static" folder name is not special. It is configured in
ember-cli-build.js. Alternatively, we could have left everything
in their normal locations, and add more fine-grained paths to the
`staticAppPaths` array. I just thought it would be easy to manage
and scale, and less error-prone to do it this way.
Note that putting things in `app/static` does not guarantee that
it would not be part of the main app bundle. For example, if we
were to add an `import ... from "app/static/wizard/...";` in a
main bundle file (say, `app.js`), then that chunk of the module
graph would be pulled in. (Consider using `await import(...)`?)
Overtime, we can build better tooling (e.g. lint rules and babel
macros to make things less repetitive) as we expand the use of
this pattern, but this is a start.
Co-authored-by: Godfrey Chan <godfreykfc@gmail.com>
The regen_ember_5_lockfile script was actually just duplicating the ember3 lockfile without changes 🤦♂️. This commit fixes that, and updates the ember-version-enforcement workflow to detect lockfile issues in future.
Consumers should use the default export. This function doesn't work directly (unless you manually construct its arguments) - the default export helper handles all that automatically.
This makes it much easier to see what a production site will look like before launch. The notices return on the next pageload, so there is minimal risk of this affecting visibility of an email configuration problem.
This new navbar component is used for every navbar in chat, full page or drawer, and any screen.
This commit also uses this opportunity to correctly decouple drawer-routes from full page routes. This will avoid having this kind of properties in components: `@includeHeader={{false}}`. The header is now defined in the parent template using a navbar. Each route has now its own template wrapped in a div of the name of the route, eg: `<div class="c-routes-threads">..</div>`.
The navbar API:
```gjs
<Navbar as |navbar|>
<navbar.BackButton />
<navbar.Title @title="Foo" />
<navbar.ChannelTitle @channel={{@channel}} />
<navbar.Actions as |action|>
<action.CloseThreadButton />
</navbar.Actions>
</navbar>
```
The full list of components is listed in `plugins/chat/assets/javascripts/discourse/components/navbar/index.gjs` and `plugins/chat/assets/javascripts/discourse/components/navbar/actions.gjs`.
Visually the header is not changing much, only in drawer mode the background has been removed.
This commit also introduces a `<List />` component to facilitate rendering lists in chat plugin.
This commit refactor CategoryList to remove usage of EmberObject,
hopefully make the code more readable and fixes various edge cases with
lazy loaded categories (third level subcategories not being visible,
subcategories not being visible on category page, requesting for more
pages even if the last one did not return any results, etc).
The problems have always been here, but were not visible because a lot
of the processing was handled by the server and then the result was
serialized. With more of these being moved to the client side for the
lazy category loading, the problems became more obvious.
Previously, `addGlobalNotice` would have to be called before the GlobalNotice component was rendered. By using a TrackedArray, we can improve that so that plugins can call the function at any time and the notice will be rendered immediately
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the min_trust_level_to_allow_ignore site setting to ignore_allowed_groups.
This PR maintains backwards compatibility until we can update plugins and themes using this.
This is v0 of admin sidebar navigation, which moves
all of the top-level admin nav from the top of the page
into a sidebar. This is hidden behind a enable_admin_sidebar_navigation
site setting, and is opt-in for now.
This sidebar is dynamically shown whenever the user enters an
admin route in the UI, and is hidden and replaced with either
the:
* Main forum sidebar
* Chat sidebar
Depending on where they navigate to. For now, custom sections
are not supported in the admin sidebar.
This commit removes the experimental admin sidebar generation rake
task but keeps the experimental sidebar UI for now for further
testing; it just uses the real nav as the default now.
This bug appears to only be on Chrome due to the service worker fetching
the video content on page load instead of on play. For some reason
though the service worker would fetch around 4x more than the size of
the video resulting in excessive data being downloaded especially for
larger videos.
meta https://meta.discourse.org/t/287817
internal /t/111387/52
Float-kit elements (menus/tooltips) are positioned where they should be by setting an inline `left` property in JavaScript when they're rendered. For some reasons, we also set `left: 0` on float-kit elements here:
25d9927785/app/assets/stylesheets/common/float-kit/d-menu.scss (L11-L15)
This property is overridden by the inline property that the library sets in JavaScript. However, in RTL mode, all of our scss files are flipped where everything left becomes right and vice versa. In this case, the `left: 0` property in the scss file above becomes `right: 0`.
This results in a conflict specific to RTL mode where both the `left` and `right` properties are defined on the same absolute-positioned element; the `right` property will always be set to 0 because it comes from the (flipped) scss file above, and the inline `left` property will be set to some px amount determined in JavaScript.
The `right` property will take precedence over the inline `left` property due to the page being right-to-left (source: https://developer.mozilla.org/en-US/docs/Web/CSS/right#description) and this causes float-kit elements to incorrectly always stick to the right.
This commit removes the `left: 0` property altogether for float-kit elements from our scss files. It's not clear from git history why the property was added, and removing it doesn't seem to cause any issues.
Meta topic: https://meta.discourse.org/t/positioning-issues-with-rtl-locales-after-recent-updates/280220?u=osama
We're changing the implementation of trust levels to use groups. Part of this is to have site settings that reference trust levels use groups instead. It converts the tl4_delete_posts_and_topics site setting to delete_all_posts_and_topics_allowed_groups.
This one is a bit different from previous ones, as it's a boolean flag, and the default should be no group. Pay special attention to the migration during review.
This commit adds an additional toggle to our safe-mode system. When enabled, it will cause all deprecation messages to become exceptions. This gives admins a way to test their themes/plugins against upcoming Discourse changes without needing to use the browser developer tools.
This change converts the allow_uploaded_avatars site setting to uploaded_avatars_allowed_groups.
See: https://meta.discourse.org/t/283408
Hides the old setting
Adds the new site setting
Adds a deprecation warning
Updates to use the new setting
Adds a migration to fill in the new setting if the old setting was changed
Adds an entry to the site_setting.keywords section
Updates tests to account for the new change
After a couple of months, we will remove the allow_uploaded_avatars setting entirely.
Internal ref: /t/117248
Now that we're using native `import()`, our main JS bundles might not even be parse-able by older browsers. In that case, `I18n` will never be defined, and so we need to account for that situation in the browser-update code.
In modern hljs, languages should be targetted with `lang-` prefixes. These selectors haven't worked in Discourse for a long time, so let's drop them to reduce confusion
We no longer offer the option to use the legacy hamburger menu since October 9th 2023, see 832b3b9e60. However, the code for the legacy hamburger menu is still around and needs to be removed. All plugins and themes that we know of that customize the legacy hamburger menu have been updated to either remove the customizations or migrate the customizations to the new sidebar, so now we can safely remove the legacy hamburger menu code from core.
Internal topic: t/113137.
When `lazy_load_categories` is enabled, the categories are no longer
preloaded in the `Site` object, but instead they are being requested
on a need basis.
The categories page still loaded all categories at once, which was not
ideal for sites with many categories because ti would take a lot of
time to build and parse the response.
This commit adds pagination to the categories page using the LoadMore
helper. As the user scrolls through the categories page, more categories
are requested from the server and appended to the page.
<!-- 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. -->
When 2FA is enforced and the user has no key or TOTP on their account, we
block navigating away from the page until they have added one. However,
we don't reload the page after they have added one, so the user is left
with a page that still says they need to add 2FA.
It's possible for browser extensions to trigger JS errors and deprecation warnings. That can lead to significant confusion and noise in our logs/metrics. One recent example we've identified is the 'Wappalyzer' extension triggering the `ember-global` deprecation.
This commit will clearly identify these errors/deprecations with a `[BROWSER EXTENSION]` prefix in the console.
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
