The commit fcc2e7ebbf to promote
polymorphic bookmarks did not correctly set the username for
the quick access bookmark menu based on the new serializer
values, so the username is not being shown in the bookmark
quick access menu. This commit fixes it, and also adds additional
tests for that menu and updates the user fixtures to reflect
the current state of the bookmarks endpoint.
Given this html:
```
<aside class="quote no-group">
<blockquote>
<aside class="quote no-group">
<blockquote>
<p dir="ltr">test</p>
</blockquote>
</aside>
<p dir="ltr">test2</p>
</blockquote>
</aside>
```
The result was an invalid markdown:
```
[quote]
[quote]
> test
> [/quote]
>
>
>
> test2
[/quote]
```
Now the result is:
```
[quote]
[quote]
test
[/quote]
test2
[/quote]
```
The bookmarkable_type instead of the bookmarkable_url
was being used for the link to the bookmark for the quick
access menu, leading to links like /ChatMessage. This
fixes the issue, follow up PR with tests for the quick
access menu to follow.
Looking up values from the `emojiStore` calls out to the browser's localStorage API and then decodes a JSON blob. This makes it relatively slow.
Previously we were doing this lookup in the emoji-picker's `init()` function, even if `isActive` was false. If many inactive emoji pickers are rendered simultaneously (e.g. for discourse-chat reactions), this performance hit quickly adds up.
This commit updates the service to notify about changes, and uses a computed property to provide a cached value in the emoji-picker.
`run-qunit.js` does not expect QUnit tests to start automatically but
our wizard QUnit setup did not respect the `qunit_disable_auto_start`
URL param. Hence, tests would start running automatically and when a
subsequent `QUnit.start()` function call is made, we ended up getting a
`QUnit.start cannot be called inside a test context.` error.
This error can be consistently reproduced in the `discourse:discourse_test` container but not in
the local development environment. I do not know why and did not feel
like it is important at this point in time to know why.
There is no need for the extra protection on the client side if there is
a bug on the server side. In fact, we want the bug to be surfaced so
that it can be fixed on the server side.
Sometimes we need to render the icon as a call to action
to create a bookmark at which point the bookmark does
not yet exist, so we need to just show the normal bookmark
icon and a create title.
Also adds a CSS class for the bookmark existing and not existing
for styling.
This improves the bookmark-icon title to be more like the
post bookmark icons, to include the special formatted date
as well as the name of the bookmark.
This component will be useful for chat, and also moves
the definition of the icon for with and without reminders
to the bookmark model as consts, so they can easily be
referenced in other places.
This commit migrates all bookmarks to be polymorphic (using the
bookmarkable_id and bookmarkable_type) columns. It also deletes
all the old code guarded behind the use_polymorphic_bookmarks setting
and changes that setting to true for all sites and by default for
the sake of plugins.
No data is deleted in the migrations, the old post_id and for_topic
columns for bookmarks will be dropped later on.
Previously we were only applying the restriction to `a[href]` and `img[src]`. This commit ensures we apply the same logic to all allowlisted media src attributes.
Categories that had a CategoryUser record and the notification level
set to "Normal" were not selectable in any of the "Watched", "Tracked",
"Watching First Post" or "Muted" inputs. This happened because the
category seemed to be already selected in the "Normal" input, but that
does not exist (it is the default value if category is not present in
any of the other inputs).
The composer is displayed over the bottom part of the page. To make sure
that no content is covered by the composer, a bottom padding is added
equal to the height of the composer. When the composer is opened or
closed that padding is added after around 300ms because of a debounce.
This commit makes sure that the padding is added as soon as the composer
state changes by using a CSS custom property (variable) and transition
property for a smooth user interface.
The title had to be added both on the 404 page generated by the server
side, displayed when the user reaches a bad page directly and the 404
page rendered by Ember when a user reaches a missing topic while
navigating the forum.
If userA has multiple tab/devices on the same topic, and:
1. userA likes a post in tab1
2. userB likes the same post
3. userA post like `acted` attr would desync in tab2
This fix handles this case and also the reverse one when removing likes
interleaved with other users acting on the same post.
Reported in Meta at https://meta.discourse.org/t/-/227239/3
cf273ec6 removed ie11 as a target. A side effect is that this also removed support for Safari 12, which we will be maintaining support for until January 2023
https://meta.discourse.org/t/224747
While editing a reviewable's tags, the tag chooser did not show the tags
restricted to a specific category. This happened because the tag-chooser
did not pass the categoryId to the server while it was requesting the
list of tags the user can use.
For some pages, when navigating to a topic and then pressing the Back
browser button to go back to the topic list, the scroll position was
not preserved and the user was taken to the beginning of the list.
This happened because the application failed to detect when the user
used the Back button and whether the topic list should be fetch from
the cache or not. The scroll position is preserved only for cached
topic lists.
Other improvements:
* Improve isPoppedState
* Reset position for topic-lists from user-activity page
* Remove usage of jQuery
* Make sure the scrollTo function has effect
Follow up to 618a1ba571.
This fixes a corner case of the perf optimization in d4e35f5.
When you have the the same post showing in multiple tab/devices and like
said post in one place, we updated the like count but didn't flip the
`acted` bool in the front-end. This caused a small visual desync.
Co-authored-by: Penar Musaraj <pmusaraj@gmail.com>
The prioritize_username_in_ux site settings controls if the username or
name will be prioritized in the user interface. On the user directory
page the name was never displayed if the user and username were very
similar, being completely different from all the other places where the
username or name is displayed.
The scroll position was reset everytime the user pressed the back button
and returned to a user activity page. This fix applies only to pages
that uses the user-stream component. Pages with topic lists already had
this functionality implemented.
A bit of a mixed bag, this addresses several edge areas of bookmarks and makes them compatible with polymorphic bookmarks (hidden behind the `use_polymorphic_bookmarks` site setting). The main ones are:
* ExportUserArchive compatibility
* SyncTopicUserBookmarked job compatibility
* Sending different notifications for the bookmark reminders based on the bookmarkable type
* Import scripts compatibility
* BookmarkReminderNotificationHandler compatibility
This PR also refactors the `register_bookmarkable` API so it accepts a class descended from a `BaseBookmarkable` class instead. This was done because we kept having to add more and more lambdas/properties inline and it was very messy, so a factory pattern is cleaner. The classes can be tested independently as well.
Some later PRs will address some other areas like the discourse narrative bot, advanced search, reports, and the .ics endpoint for bookmarks.
Note this commit also introduce a new {{d-popover}} component, example usage:
```hbs
{{#d-popover |state|}}
{{d-button label="foo.things" class="d-popover-trigger"}}
<div class="d-popover-content">
Some content
<div>
{{/d-popover}}
```
If you happen to mention someone with the wrong capitalization for that
person's username (e.g. `@sAm`), that incorrect capitalization would get
displayed on their user card.
This update will fix that by displaying the `user.username` value, which
will have the correct capitalization.
I also added some tests that will ensure `username` and `name` are
displayed on the user card in the correct order based on the
`prioritize_username_in_ux` setting.
This issue was reported here:
https://meta.discourse.org/t/capitalization-does-not-match-when-you-open-user-cards-using-mentions/217166
The values in Discourse dropdown menus only come from admin-defined strings, not unsanitised end-user input, so this lack of escaping was not exploitable.
* hidden siteSetting to enable experimental sidebar
* user preference to enable experimental sidebar
* `experimental_sidebar_enabled` attribute for current user
* Empty glimmer component for Sidebar
Before this change, we were using the labels from the original chartData to the chart builder, and we would then apply our collapse function on each dataset which could change the labels and cause a mismatch.
This was very visible when using quarterly periods on consolidated pageviews.
Other things may have added things to the style attribute (e.g. the `image-aspect-ratio` decorator).
Unfortunately this is difficult to add a test for because `lazy-load-images` leans on the `onload` event. We have no control over image loading in tests, so race conditions would be very likely.
- Make proxy pass `x-forward...` headers, so that Rails can set the host/port correctly in the csp
- Make `testem.js` available on a route which is within the app's default CSP
There was an edge when a user re-quoted a nested quote that it would return an incorrect `full name` but the correct `username` for the original quote. This PR updates the logic to fall back to using the OP user's username.
The complexity of the changes required to allow for full names to be displayed on nested quotes far outweighs how rare quoting nested quotes is.
This pull request follows on from https://github.com/discourse/discourse/pull/16308. This one does the following:
* Changes `BookmarkQuery` to allow for querying more than just Post and Topic bookmarkables
* Introduces a `Bookmark.register_bookmarkable` method which requires a model, serializer, fields and preload includes for searching. These registered `Bookmarkable` types are then used when validating new bookmarks, and also when determining which serializer to use for the bookmark list. The `Post` and `Topic` bookmarkables are registered by default.
* Adds new specific types for Post and Topic bookmark serializers along with preloading of associations in `UserBookmarkList`
* Changes to the user bookmark list template to allow for more generic bookmarkable types alongside the Post and Topic ones which need to display in a particular way
All of these changes are gated behind the `use_polymorphic_bookmarks` site setting, apart from the .hbs changes where I have updated the original `UserBookmarkSerializer` with some stub methods.
Following this PR will be several plugin PRs (for assign, chat, encrypt) that will register their own bookmarkable types or otherwise alter the bookmark serializers in their own way, also gated behind `use_polymorphic_bookmarks`.
This commit also removes `BookmarkQuery.preloaded_custom_fields` and the functionality surrounding it. It was added in 0cd502a558 but only used by one plugin (discourse-assign) where it has since been removed, and is now used by no plugins. We don't need it anymore.
This was causing unexpected behaviors on production builds. And also on firefox on local environnement, however the issues was slightly different.
- production chrome: colors don't load
- dev firefox: colors don't change when selecting a different color set
Previously, accessing the Rails app directly in development mode would give you assets from our 'legacy' Ember asset pipeline. The only way to run with Ember CLI assets was to run ember-cli as a proxy. This was quite limiting when working on things which are bypassed when using the ember-cli proxy (e.g. changes to `application.html.erb`). Also, since `ember-auto-import` introduced chunking, visiting `/theme-qunit` under Ember CLI was failing to include all necessary chunks.
This commit teaches Sprockets about our Ember CLI assets so that they can be used in development mode, and are automatically collected up under `/public/assets` during `assets:precompile`. As a bonus, this allows us to remove all the custom manifest modification from `assets:precompile`.
The key changes are:
- Introduce a shared `EmberCli.enabled?` helper
- When ember-cli is enabled, add ember-cli `/dist/assets` as the top-priority Rails asset directory
- Have ember-cli output a `chunks.json` manifest, and teach `preload_script` to read it and append the correct chunks to their associated `afterFile`
- Remove most custom ember-cli logic from the `assets:precompile` step. Instead, rely on Rails to take care of pulling the 'precompiled' assets into the `public/assets` directory. Move the 'renaming' logic to runtime, so it can be used in development mode as well.
- Remove fingerprinting from `ember-cli-build`, and allow Rails to take care of things
Long-term, we may want to replace Sprockets with the lighter-weight Propshaft. The changes made in this commit have been made with that long-term goal in mind.
tldr: when you visit the rails app directly, you'll now be served the current ember-cli assets. To keep these up-to-date make sure either `ember serve`, or `ember build --watch` is running. If you really want to load the old non-ember-cli assets, then you should start the server with `EMBER_CLI_PROD_ASSETS=0`. (the legacy asset pipeline will be removed very soon)
Commit 68497bddf2 implemented a function
to format durations in a medium format, similar to how durationTiny did.
The existent translation strings do not cover all cases and this commit
adds the missing translation strings.
1. When the select-kit body is rendered, it defaults to being displayed under the triggering select-kit header, unless...
there isn't enough space between the bottom of the select-kit header and the bottom of the viewport
&
there's enough space on top of the select-kit header, and in that case, we render it on top.
2. We give it a bit of padding on top, so it never renders below the header on the Z-axis.
14778ba52e/app/assets/javascripts/select-kit/addon/components/select-kit.js (L877-L884)
3. If there isn't enough space between the bottom of the viewport and the bottom of the select-kit header, and there isn't enough space between its top and the bottom of `d-header`, it renders at the bottom of the select-kit header.
In theory, number 3 above rarely ever happens. However, it can occur in the case of the user preferences page in combination with a large select-kit body (many categories).
The select-kit body then renders below the trigging select-kit header, but it's cut off. Users won't be able to see the entire select-kit body.
Here's an example
a719734d92.mp4
This PR adds a "prevent overflow" modifier to Popper. What it does is that it handles the case above.
If there's not enough space below the select-kit header or above it, render the select-kit body below the select-kit header BUT... anchor it to the bottom of the viewport.
Here's what that looks like
32cd1639bb.mp4
After this fix, even very large select-kit bodies will always be on the screen.
Please note that this PR has no impact on either number 1 or number 2 above, and those will continue to function as they currently do.
The only downside here is that the select-kit body might cover the select-kit header if it needs to be anchored at the bottom of the viewport, and it's very large. However, between that and not being able to see all the options, I think it's a fair compromise. There's only so much space in the viewport.
This PR ignores mobile because we have a different placement strategy. We use `position: absolute`... so, users can scroll the viewport if needed.
This commit fixes two issues at play. The first was introduced
in f6c852b (or maybe not introduced
but rather revealed). When a user posted a new message in a topic,
they received the unread topic tracking state MessageBus message,
and the Unread (X) indicator was incremented by one, because with the
aforementioned perf commit we "guess" the correct last read post
for the user, because we no longer calculate individual users' read
status there. This meant that every time a user posted in a topic
they tracked, the unread indicator was incremented. To get around
this, we can just exclude the user who created the post from the
target users of the unread state message.
The second issue was related to the private message topic tracking
state, and was somewhat similar. Whenever a user created a new private
message, the New (X) indicator was incremented, and could not be
cleared until the page was refreshed. To solve this, we just don't
update the topic state for the user when the new_topic tracking state
message comes through if the user who created the topic is the
same as the current user.
cf. https://meta.discourse.org/t/bottom-of-topic-shows-there-is-1-unread-remaining-when-there-are-actually-0-unread-topics-remaining/220817
This updates the fix in commit eb70ea4.
Co-authored-by: Osama Sayegh <asooomaasoooma90@gmail.com>
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
This PR will include `suspended` attribute in post serializer to check it in post widget and add a CSS class name.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
The error would happen when emoji_autocomplete_min_chars site setting is set to anything superior to 0, in this case until we reach the min chars length, emojiSearch would return "skip" and the code was currently expecting an array.
Discourse has the Discourse Connect Provider protocol that makes it possible to
use a Discourse instance as an identity provider for external sites. As a
natural extension to this protocol, this PR adds a new feature that makes it
possible to use Discourse as a 2FA provider as well as an identity provider.
The rationale for this change is that it's very difficult to implement 2FA
support in a website and if you have multiple websites that need to have 2FA,
it's unrealistic to build and maintain a separate 2FA implementation for each
one. But with this change, you can piggyback on Discourse to take care of all
the 2FA details for you for as many sites as you wish.
To use Discourse as a 2FA provider, you'll need to follow this guide:
https://meta.discourse.org/t/-/32974. It walks you through what you need to
implement on your end/site and how to configure your Discourse instance. Once
you're done, there is only one additional thing you need to do which is to
include `require_2fa=true` in the payload that you send to Discourse.
When Discourse sees `require_2fa=true`, it'll prompt the user to confirm their
2FA using whatever methods they've enabled (TOTP or security keys), and once
they confirm they'll be redirected back to the return URL you've configured and
the payload will contain `confirmed_2fa=true`. If the user has no 2FA methods
enabled however, the payload will not contain `confirmed_2fa`, but it will
contain `no_2fa_methods=true`.
You'll need to be careful to re-run all the security checks and ensure the user
can still access the resource on your site after they return from Discourse.
This is very important because there's nothing that guarantees the user that
will come back from Discourse after they confirm 2FA is the same user that
you've redirected to Discourse.
Internal ticket: t62183.
All current browser treat the HTML document (not the body element) as
the scrollable document element. Hence in all current browsers,
`document.body.scrollTop` returns 0. This commit removes all usage of
this property, because it is effectively 0.
Co-authored-by: David Taylor <david@taylorhq.com>
Firefox does not support window.ClipboardItem yet (it is behind
a flag (dom.events.asyncClipboard.clipboardItem) as at version 87.)
so we need to fall back to the normal non-async clipboard copy, that
works in every browser except Safari.
This commit also tests the clipboardCopyAsync function by stubbing out
the clipboard on the window.navigator.
This fixes an issue in the discourse-chat plugin, where the
"Quote in Topic" button errored in Firefox.
After this commit, category group permissions can only be seen by users
that are allowed to manage a category. In the past, we inadvertently
included a category's group permissions settings in `CategoriesController#show`
and `CategoriesController#find_by_slug` endpoints for normal users when
those settings are only a concern to users that can manage a category.
These were originally very similar, but have diverged over time. This makes it very difficult to manage styling.
This commit moves the noscript header and footer into partials so they can be reused in both the crawler view and the `<noscript>` view. It also makes browser-update render the noscript content **instead of** the `<section id='main'>`, rather than adding adding the noscript inside the `<section>`. This provides better parity with the server-rendered crawler view.
The test was dependent on a translation string. Under certain seeds, the translation string for `{{category-drop}}`'s `noCategoriesLabel` is broken. This is because the value is calculated the first time a `{{category-drop}}` is rendered during the suite. If that first time happens to be during a test which is messing with `I18n.translations`, then it will cache a broken value. Maybe this should be fixed in a future commit... but for now moving to `data-value` will make the `tags-test` more robust and will stop the flakiness.
Moderators don't have access to notifications of other users. So we shouldn't display the notifications tab on other user profiles for them.
Co-authored-by: Alan Guo Xiang Tan <gxtan1990@gmail.com>
Follow up to ac672cf. Fixes a
small issue with uppy-image-uploader where the Processing label
was shown for the whole upload. Also adds a couple of options to
pick-files-button to allow for it to be used in the uppy-image-uploader.
Also fixes an issue where the uppy-upload mixin was resetting prematurely
when all uploads in progress were complete, but it should have been doing
that on the uppy complete event instead.
This PR brings the `UppyUploadMixin` more into line with the `ComposerUppyUpload` mixin, by extending the `ExtendableUploader` . This also adds better tracking of and events for in progress uploads in the `UppyUploadMixin` for better UI interactions, and also opens up the use of `_useUploadPlugin` for the mixin, so anything implementing `UppyUploadMixin` can add extra uppy preprocessor plugins as needed.
This has been done as part of work on extracting uploads out of the chat composer. In future, we might be able to do the same for `ComposerUppyUpload`, getting rid of that mixin to standardise on `UppyUploadMixin` and have a separate `composer-uploads` component that lives alongside `composer-editor` like what we are doing in https://github.com/discourse/discourse-chat/pull/764
There are still some, but those are in actual code that's used outside core, so the change there would need to go through the deprecation cycle. That's a task for another day.
Previously we only supported a single 'required tag group' for a category. This commit allows admins to specify multiple required tag groups, each with their own minimum tag count.
A new category_required_tag_groups database table replaces the existing columns on the categories table. Data is automatically migrated.
A while ago in 27b97e4 the
pick-files-input was added but only used once for data-explorer. This commit uses it
for the composer-editor, and cleans it up to be usable either via uppy
handling the uploads or with this component handling the uploads.
This can then be used in other places in the app and also for plugins.
Browsers automatically calculate an aspect ratio based on the width/height attributes of an `<img`. HOWEVER that aspect ratio only applies while the image is loading. Once loaded, it'll use the image's actual dimensions. This can cause things to jump around after loading. For example:
- if a user deliberately inserts false width/height
- the image fails to load (404)
- an optimised image is a few pixels different, due to a rounding when resizing
This decorator explicitly sets the `aspect-ratio` property so that things are consistent throughout the lifetime of all `<img` elements.
Invited users were allowed to accept invites without entering a
password. When this happened, instead of receiving an activation email,
they received a password reset email. Basically, a user could postpone
choosing a password until after registration.
Unfortunately, this led to a confusing user experience and this commit
attempts to fix that by making the client require a password. There is
a single case when users do not need to input a password: when they sign
up using an external authenticator and password field is completely
hidden. In this case, the third party handles the password logic.
Technically, invites can still be redeemed without a password, but that
functionality was kept to preserve backwards compatibility.
`reject` method for `Reviewable` model is returning an array. So if we use `this.set` method to update `reviewables` attribute in controller then it replaces the model with an array of objects wrongly. This is now fixed by using the `setObjects` method of the model.
String.prototype.substr() is deprecated so we replace it with String.prototype.slice() which works similarily but isn't deprecated.
Signed-off-by: Tobias Speicher <rootcommander@gmail.com>
Co-authored-by: Jarek Radosz <jradosz@gmail.com>
The links returned by post.url and topic.url are relative, but contain
the subdirectory. When getAbsoluteURL is called to construct the
complete share URL, it adds the host and the subdirectory again. As a
result the created URLs contained the subdirectory twice.
The user avatar in posts has aria-hidden set to true to reduce
redundancy since the information that the avatar gives to screen readers
is the same information that the username/name of the post gives which
is the author of the post.
However, it's still possible for a screen reader user to reach the
avatar by tabbing through the post and when that happens the avatar is
read as "blank". This isn't ideal so we should set tabindex to -1 on the
avatar to remove it from the default keyboard nav flow.
The changes are:
* Add an aria-label for the button that embeds/expand the replies of a
post below it
* Add an aria-label for the button that collapses the embedded replies
* Add an aria-label to describe the embedded replies section when
expanded and an aria-label for each embedded reply
The improvements are:
* Add an aria-label to the like/read count buttons below posts to
indicate what they mean and do.
* Add aria-pressed to the like/read count buttons to make it clear to screen
readers that these buttons are toggleable.
* Add an aria-label to the list of avatars that's shown when post likes
or readers are expanded so that screen reader users can understand what
the list of avatars means.
The tabLoc is a hidden element inside the post region that we use to
move the focus close to the post that's visually highlighted (by
changing the background color and then fading it away) when a topic is
opened so that screen readers can start reading from that post rather
than the top of the page.
Some screen readers get confused by the tabLoc element being an `<a>`
element and read out the topic ID and I've found that changing the tag
to `<span>` fixes the problem.
More precisely, if popper can't position something at the bottom, it will automatically attempt to position it at the top. However we should ensure it doesn’t consider the space under the d-header as valid space, when header's height is taken into consideration if top space is not enough, we should force bottom, and flip it back.
This logic is not necessary on modals as the d-header is not present.
This commit introduces a new use_polymorphic_bookmarks site setting
that is default false and hidden, that will be used to help continuous
development of polymorphic bookmarks. This setting **should not** be
enabled anywhere in production yet, it is purely for local development.
This commit uses the setting to enable create/update/delete actions
for polymorphic bookmarks on the server and client side. The bookmark
interactions on topics/posts are all usable. Listing, searching,
sending bookmark reminders, and other edge cases will be handled
in subsequent PRs.
Comprehensive UI tests will be added in the final PR -- we already
have them for regular bookmarks, so it will just be a matter of
changing them to be for polymorphic bookmarks.
`keyUp` is only invoked if the component's root element (or one of its descendants) has focus which isn't great for keyboard users because if they open a user card and want to close it, they have to tab through to the user card and only then will the Esc key actually close the card. This commit adds a `keyup` event listener on the `document` for the Esc key so that the user card is closed (if it's open) no matter where the focus is.
Since 3fd7b31a2a some tests
were failing with this error:
> Error: Unhandled request in test environment: /c/feature/find_by_slug.json
> (GET) at http://localhost:7357/assets/test-helpers.js
This commit fixes the issue by adding the missing pretender. Also
noticed while fixing this that the parameter for the translation
was incorrect -- it was `group` instead of `groupNames`, so that
is fixed here too, along with moving the onShow functions into
@afterRender decorated private functions. There is no need for the
appevent listeners.
Previously, our `upload://` protocol urls were only supported in markdown image tags. This meant that our PullHotlinkedImages job was forced to convert `<img` tags to markdown. Depending on the exact syntax, this can actually cause the image to break.
This commit adds support for `upload://` inside regular HTML `<img` tags. In a future commit, we'll be able to use this to make our PullHotlinkedImages job much more robust.
Context at https://meta.discourse.org/t/152801
Since 6a5ef27, we made public
versions of some TextareaTextManipulation methods. This commit removes
the old underscore versions of these methods:
_focusTextArea
_insertBlock
_insertText
_getSelected
_selectText
_replaceText
_applySurround
_addText
_extractTable
_isInside
* FIX: Redirect if Discourse-Xhr-Redirect is present
`handleRedirect` was passed an wrong argument type (a string) instead of
a jqXHR object and missed the fields checked in condition, thus always
evaluating to `false`.
* FIX: Add `errors` field if group update confirmation
An explicit confirmation about the effect of the group update is
required if the default notification level changes. Previously, if the
confirmation was missing the API endpoint failed silently returning
a 200 response code and a `user_count` field. This change ensures that
a proper error code is returned (422), a descriptive error message and
the additional information in the `user_count` field.
This commit also refactors the API endpoint to use the
`Discourse-Xhr-Redirect` header to redirect the user if the group is
no longer visible.
Clicking the Replies cell of a topic in a topics list shows a little
modal with 2 buttons that take you to the first and last posts of the
topic. This modal is currently completely inaccessible to
keyboard/screen reader users because it can't be reached using the
keyboard.
This commit improves the modal so that it traps focus when it's shown
and makes it possible to close the modal using the esc key.
Topics lists like /latest are ordered by last activity date by default,
but the order can be changed (and reversed) to something else such as
replies count and views count by clicking on the corresponding column
header in the topics list. These column headers are tabbable, but screen
readers announce them as, using the replies column as example, `Replies
toggle button`. This doesn't communicate very well that this the button
changes the order, so this commit adds `aria-label`s to all column
headers to make it clear that they change order. The current copy for
the `aria-label` is `Sort by replies`.
When tabbing through a topics list like /latest, /unread, /new etc. the
Replies column is announced as `<replies count> button` by screen
readers and it's not clear that number means the topic has that number
of replies. This commit adds an `aria-label` so the Replies column to
make it clear what that number means. The current copy of the
`aria-label` is "This topic has <replies count> replies".
Follow-up to 97e7bb1ce4
Themes/plugins may override the default `topic-list-item` and remove the `.main-link` or `.title` elements from the template. We shouldn't attempt to focus them if they don't exist.
