-
-
Notifications
You must be signed in to change notification settings - Fork 864
[2.x] feat(core, mentions): implement ability to order groups #4350
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 2.x
Are you sure you want to change the base?
Changes from all commits
5d9fe75
af9f285
acccd87
325db3f
37cc6e1
8af4b74
75c54a5
fdc1557
1996a8f
fc0ad20
8f7c857
f014959
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| import sortable from 'sortablejs'; | ||
|
|
||
| import app from '../../admin/app'; | ||
| import Component, { ComponentAttrs } from '../../common/Component'; | ||
| import GroupBadge from '../../common/components/GroupBadge'; | ||
| import Icon from '../../common/components/Icon'; | ||
| import EditGroupModal from './EditGroupModal'; | ||
| import sortGroups from '../../common/utils/sortGroups'; | ||
|
|
||
| import type Group from '../../common/models/Group'; | ||
| import type Mithril from 'mithril'; | ||
|
|
||
| export interface IGroupBarAttrs extends ComponentAttrs { | ||
| groups: Group[]; | ||
| } | ||
|
|
||
| export default abstract class GroupBar<CustomAttrs extends IGroupBarAttrs = IGroupBarAttrs> extends Component<CustomAttrs> { | ||
| groups: Group[] = []; | ||
|
|
||
| oninit(vnode: Mithril.Vnode<CustomAttrs, this>) { | ||
| super.oninit(vnode); | ||
|
|
||
| this.groups = sortGroups(this.attrs.groups); | ||
| } | ||
|
|
||
| view(): JSX.Element { | ||
| return ( | ||
| <div className="GroupBar" oncreate={this.onGroupBarCreate.bind(this)}> | ||
| {this.groups.map((group) => ( | ||
| <button className="Button Group" type="button" data-id={group.id()} onclick={() => app.modal.show(EditGroupModal, { group })}> | ||
| <GroupBadge group={group} className="Group-icon" label={null} /> | ||
| <span className="Group-name">{group.namePlural()}</span> | ||
| </button> | ||
| ))} | ||
| <button className="Button Group Group--add" type="button" onclick={() => app.modal.show(EditGroupModal)}> | ||
| <Icon name="fas fa-plus" className="Group-icon" /> | ||
| <span className="Group-name">{app.translator.trans('core.admin.permissions.new_group_button')}</span> | ||
| </button> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| onGroupBarCreate(vnode: Mithril.VnodeDOM) { | ||
| sortable.create(vnode.dom as HTMLElement, { | ||
| group: 'groups', | ||
| delay: 50, | ||
| delayOnTouchOnly: true, | ||
| touchStartThreshold: 5, | ||
| animation: 150, | ||
| swapThreshold: 0.65, | ||
| dragClass: 'Group-Sortable-Dragging', | ||
| ghostClass: 'Group-Sortable-Placeholder', | ||
|
|
||
| filter: '.Group--add', | ||
| onMove: (evt) => !evt.related.classList.contains('Group--add'), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
onSort: () => this.onSortUpdate(), |
||
|
|
||
| onSort: (e) => this.onSortUpdate(), | ||
| }); | ||
| } | ||
|
|
||
| onSortUpdate() { | ||
| const order = this.$('.Group:not(.Group--add)') | ||
| .map(function () { | ||
| return $(this).data('id'); | ||
| }) | ||
| .get(); | ||
|
|
||
| order.forEach((id, i) => { | ||
| app.store.getById<Group>('groups', id)?.pushData({ | ||
| attributes: { position: i }, | ||
| }); | ||
| }); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
At minimum, revert the store positions on failure: const previousPositions = order.map((id, i) => ({
id,
position: app.store.getById<Group>("groups", id)?.position() ?? null,
}));
app.request({ ... }).catch(() => {
previousPositions.forEach(({ id, position }) => {
app.store.getById<Group>("groups", id)?.pushData({ attributes: { position } });
});
m.redraw();
}); |
||
|
|
||
| app.request({ | ||
| url: app.forum.attribute('apiUrl') + '/groups/order', | ||
| method: 'POST', | ||
| body: { order }, | ||
| }); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| import type Group from '../models/Group'; | ||
|
|
||
| export default function sortGroups(groups: Group[]) { | ||
| return groups.slice().sort((a, b) => a.position() - b.position()); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will behave incorrectly when Please handle null positions explicitly, following the same pattern as export default function sortGroups(groups: Group[]) {
return groups.slice().sort((a, b) => {
const aPos = a.position();
const bPos = b.position();
if (aPos === null && bPos === null) return 0;
if (aPos === null) return 1;
if (bPos === null) return -1;
return aPos - bPos;
});
} |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,4 +5,9 @@ module.exports = merge(config(), { | |
| output: { | ||
| library: 'flarum.core', | ||
| }, | ||
| resolve: { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This alias fixes the Terser/ESM issue for core, but any extension that bundles sortablejs directly will hit the same error. The fix should live in Could you look into moving this to |
||
| alias: { | ||
| sortablejs: require.resolve('sortablejs/Sortable.js'), | ||
| }, | ||
| }, | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,15 @@ | ||
| .PermissionsPage-groups { | ||
| .GroupBar { | ||
| background: var(--control-bg); | ||
| border-radius: var(--border-radius); | ||
| display: block; | ||
| overflow-x: auto; | ||
| display: flex; | ||
| gap: 10px; | ||
| flex-wrap: wrap; | ||
| padding: 10px; | ||
| border-radius: var(--border-radius); | ||
| } | ||
| .Group-Sortable-Placeholder { | ||
| outline: 2px dashed var(--body-bg); | ||
| border-radius: var(--border-radius); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent indentation — the rest of the file uses 2-space indent but |
||
| .Group { | ||
| width: 90px; | ||
| display: inline-block; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| <?php | ||
|
|
||
| /* | ||
| * This file is part of Flarum. | ||
| * | ||
| * For detailed copyright and license information, please view the | ||
| * LICENSE file that was distributed with this source code. | ||
| */ | ||
|
|
||
| use Illuminate\Database\Schema\Blueprint; | ||
| use Illuminate\Database\Schema\Builder; | ||
|
|
||
| return [ | ||
| 'up' => function (Builder $schema) { | ||
| $schema->table('groups', function (Blueprint $table) { | ||
| $table->integer('position')->after('is_hidden')->nullable(); | ||
| }); | ||
|
|
||
| $db = $schema->getConnection(); | ||
|
|
||
| $ids = $db->table('groups') | ||
| ->orderBy('id') | ||
| ->pluck('id'); | ||
|
|
||
| $position = 0; | ||
| foreach ($ids as $id) { | ||
| $db->table('groups') | ||
| ->where('id', $id) | ||
| ->update(['position' => $position]); | ||
|
|
||
| $position++; | ||
| } | ||
| }, | ||
|
|
||
| 'down' => function (Builder $schema) { | ||
| $schema->table('groups', function (Blueprint $table) { | ||
| $table->dropColumn('position'); | ||
| }); | ||
| } | ||
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GroupBaris declaredabstractbutPermissionsPageuses it directly via<GroupBar groups={availableGroups} />. TypeScript will compile this, but Mithril instantiates components at runtime — instantiating an abstract class throws in JS.Either:
abstractif this is meant to be directly usable as-is, orclass DefaultGroupBar extends GroupBar {}) forPermissionsPageto use — which would also give extensions a clean extension point without having to extend an abstract class directly.