chore: improvements

flarum · Feb 7, 2025 · ea089c1 · ea089c1
1 parent 9733081
commit ea089c1
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 44 deletions.
diff --git a/extensions/messages/js/src/forum/components/DialogSection.tsx b/extensions/messages/js/src/forum/components/DialogSection.tsx
@@ -29,7 +29,7 @@ export default class DialogSection<CustomAttrs extends IDialogStreamAttrs = IDia
     this.messages.refresh();
   }
 
-  requestParams(): any {
+  requestParams(forgetNear = false): any {
     const params: any = {
       filter: {
         dialog: this.attrs.dialog.id(),
@@ -39,7 +39,7 @@ export default class DialogSection<CustomAttrs extends IDialogStreamAttrs = IDia
 
     const near = m.route.param('near');
 
-    if (near) {
+    if (near && !forgetNear) {
       params.page = params.page || {};
       params.page.near = parseInt(near);
     }

diff --git a/extensions/messages/js/src/forum/components/MessageStream.tsx b/extensions/messages/js/src/forum/components/MessageStream.tsx
@@ -77,27 +77,29 @@ export default class MessageStream<CustomAttrs extends IDialogStreamAttrs = IDia
   content() {
     const items: Mithril.Children[] = [];
 
-    const messages = this.attrs.state.getAllItems().sort((a, b) => a.createdAt().getTime() - b.createdAt().getTime());
+    const messages = Array.from(new Map(this.attrs.state.getAllItems().map((msg) => [msg.id(), msg])).values()).sort(
+      (a, b) => a.number() - b.number()
+    );
 
     const ReplyPlaceholder = this.replyPlaceholderComponent();
     const LoadingPost = this.loadingPostComponent();
 
     if (messages[0].id() !== (this.attrs.dialog.data.relationships?.firstMessage.data as ModelIdentifier).id) {
       items.push(
-        <div className="MessageStream-item" key="loadPrevious">
+        <div className="MessageStream-item" key="loadNext">
           <Button
             onclick={() => this.whileMaintainingScroll(() => this.attrs.state.loadNext())}
             type="button"
-            className="Button Button--block MessageStream-loadPrev"
+            className="Button Button--block MessageStream-loadNext"
           >
-            {app.translator.trans('flarum-messages.forum.messages_page.stream.load_previous_button')}
+            {app.translator.trans('flarum-messages.forum.messages_page.stream.load_next_button')}
           </Button>
         </div>
       );
 
       if (LoadingPost) {
         items.push(
-          <div className="MessageStream-item" key="loading-prev">
+          <div className="MessageStream-item" key="loading-next">
             <LoadingPost />
           </div>
         );
@@ -106,6 +108,28 @@ export default class MessageStream<CustomAttrs extends IDialogStreamAttrs = IDia
 
     messages.forEach((message, index) => items.push(this.messageItem(message, index)));
 
+    if (messages[messages.length - 1].id() !== (this.attrs.dialog.data.relationships?.lastMessage.data as ModelIdentifier).id) {
+      if (LoadingPost) {
+        items.push(
+          <div className="MessageStream-item" key="loading-prev">
+            <LoadingPost />
+          </div>
+        );
+      }
+
+      items.push(
+        <div className="MessageStream-item" key="loadPrev">
+          <Button
+            onclick={() => this.whileMaintainingScroll(() => this.attrs.state.loadPrev())}
+            type="button"
+            className="Button Button--block MessageStream-loadPrev"
+          >
+            {app.translator.trans('flarum-messages.forum.messages_page.stream.load_previous_button')}
+          </Button>
+        </div>
+      );
+    }
+
     if (app.session.user!.canSendAnyMessage() && ReplyPlaceholder) {
       items.push(
         <div className="MessageStream-item" key="reply">
@@ -177,7 +201,7 @@ export default class MessageStream<CustomAttrs extends IDialogStreamAttrs = IDia
         return this.attrs.state.loadNext();
       }
 
-      if (this.element.scrollTop + this.element.clientHeight === this.element.scrollHeight && this.attrs.state.hasPrev()) {
+      if (this.element.scrollTop + this.element.clientHeight >= this.element.scrollHeight && this.attrs.state.hasPrev()) {
         return this.attrs.state.loadPrev();
       }
 
@@ -193,9 +217,10 @@ export default class MessageStream<CustomAttrs extends IDialogStreamAttrs = IDia
 
       if ($message) {
         this.element.scrollTop = $message.getBoundingClientRect().top - this.element.getBoundingClientRect().top;
-
-        // pulsate the message
         $message.classList.add('flash');
+
+        // forget near
+        window.history.replaceState(null, '', app.route.dialog(this.attrs.dialog));
       } else {
         this.element.scrollTop = this.element.scrollHeight;
       }
@@ -208,9 +233,11 @@ export default class MessageStream<CustomAttrs extends IDialogStreamAttrs = IDia
     const scrollTop = this.element.scrollTop;
     const scrollHeight = this.element.scrollHeight;
 
+    const closerToBottomThanTop = scrollTop > (scrollHeight - this.element.clientHeight) / 2;
+
     const result = callback();
 
-    if (result instanceof Promise) {
+    if (result instanceof Promise && !closerToBottomThanTop) {
       result.then(() => {
         requestAnimationFrame(() => {
           this.element.scrollTop = this.element.scrollHeight - scrollHeight + scrollTop;

diff --git a/extensions/messages/js/src/forum/states/MessageStreamState.ts b/extensions/messages/js/src/forum/states/MessageStreamState.ts
@@ -1,5 +1,6 @@
 import PaginatedListState, { PaginatedListParams } from 'flarum/common/states/PaginatedListState';
 import DialogMessage from '../../common/models/DialogMessage';
+import { ApiQueryParamsPlural } from 'flarum/common/Store';
 
 export interface MessageStreamParams extends PaginatedListParams {
   //

diff --git a/extensions/messages/locale/en.yml b/extensions/messages/locale/en.yml
@@ -53,6 +53,7 @@ flarum-messages:
       send_message_button: Send a Message
       stream:
         load_previous_button: Load previous messages
+        load_next_button: Load next messages
         start_of_the_conversation: Start of the conversation
         time_lapsed_text: => core.forum.post_stream.time_lapsed_text
       title: Messages

diff --git a/framework/core/js/src/common/states/PaginatedListState.ts b/framework/core/js/src/common/states/PaginatedListState.ts
@@ -1,8 +1,7 @@
 import app from '../../common/app';
-import Model from '../Model';
-import { ApiQueryParamsPlural, ApiResponsePlural } from '../Store';
+import type Model from '../Model';
+import type { ApiQueryParamsPlural, ApiResponsePlural } from '../Store';
 import type Mithril from 'mithril';
-import setRouteWithForcedRefresh from '../utils/setRouteWithForcedRefresh';
 
 export type SortMapItem =
   | string
@@ -15,9 +14,9 @@ export type SortMap = {
   [key: string]: SortMapItem;
 };
 
-export interface Page<TModel> {
+export interface Page<TModel extends Model> {
   number: number;
-  items: TModel[];
+  items: ApiResponsePlural<TModel> | TModel[];
 
   hasPrev?: boolean;
   hasNext?: boolean;
@@ -73,7 +72,7 @@ export default abstract class PaginatedListState<T extends Model, P extends Pagi
   }
 
   public loadPrev(): Promise<void> {
-    if (this.loadingPrev || this.getLocation().page === 1) return Promise.resolve();
+    if (this.loadingPrev || !this.hasPrev()) return Promise.resolve();
 
     this.loadingPrev = true;
 
@@ -140,7 +139,7 @@ export default abstract class PaginatedListState<T extends Model, P extends Pagi
       delete params.include;
     }
 
-    return app.store.find<T[]>(this.type, params).then((results) => {
+    return app.store.find<T[]>(this.type, this.mutateRequestParams(params, page)).then((results) => {
       const usedPerPage = results.payload?.meta?.perPage;
       const usedTotal = results.payload?.meta?.page?.total;
 
@@ -160,6 +159,35 @@ export default abstract class PaginatedListState<T extends Model, P extends Pagi
     });
   }
 
+  protected mutateRequestParams(params: ApiQueryParamsPlural, page: number): ApiQueryParamsPlural {
+    /*
+     * Support use of page[near]=
+     */
+    if (params.page?.near && this.hasItems()) {
+      delete params.page?.near;
+
+      const nextPage = this.location.page < page;
+
+      const offsets = this.getPages().map((page) => {
+        if ('payload' in page.items) {
+          return page.items.payload.meta?.page?.offset || 0;
+        }
+
+        return 0;
+      });
+
+      const minOffset = Math.min(...offsets);
+      const maxOffset = Math.max(...offsets);
+
+      const limit = this.pageSize || PaginatedListState.DEFAULT_PAGE_SIZE;
+
+      params.page ||= {};
+      params.page.offset = nextPage ? maxOffset + limit : Math.max(minOffset - limit, 0);
+    }
+
+    return params;
+  }
+
   /**
    * Get the parameters that should be passed in the API request.
    * Do not include page offset unless subclass overrides loadPage.

diff --git a/framework/core/src/Api/Endpoint/Index.php b/framework/core/src/Api/Endpoint/Index.php
@@ -69,7 +69,7 @@ public function query(?Closure $query): static
     protected function setUp(): void
     {
         $this->route('GET', '/')
-            ->query(function ($query, ?Pagination $pagination, Context $context): Context {
+            ->query(function ($query, ?Pagination $pagination, Context $context, array $filters, ?array $sort, int $offset, ?int $limit): Context {
                 $collection = $context->collection;
 
                 // This model has a searcher API, so we'll use that instead of the default.
@@ -81,13 +81,6 @@ protected function setUp(): void
                 if ($query instanceof Builder && $search->searchable($modelClass)) {
                     $actor = $context->getActor();
 
-                    $extracts = $this->defaultExtracts($context);
-
-                    $filters = $this->extractFilterValue($context, $extracts);
-                    $sort = $this->extractSortValue($context, $extracts);
-                    $limit = $this->extractLimitValue($context, $extracts);
-                    $offset = $this->extractOffsetValue($context, $extracts);
-
                     $sortIsDefault = ! $context->queryParam('sort');
 
                     $results = $search->query(
@@ -101,8 +94,8 @@ protected function setUp(): void
                 else {
                     $context = $context->withQuery($query);
 
-                    $this->applySorts($query, $context);
-                    $this->applyFilters($query, $context);
+                    $this->applySorts($query, $context, $sort);
+                    $this->applyFilters($query, $context, $filters);
 
                     if ($pagination && method_exists($pagination, 'apply')) {
                         $pagination->apply($query);
@@ -130,8 +123,20 @@ protected function setUp(): void
 
                 $pagination = ($this->paginationResolver)($context);
 
+                $extracts = $this->defaultExtracts($context);
+
+                $filters = $this->extractFilterValue($context, $extracts);
+                $sort = $this->extractSortValue($context, $extracts);
+                $limit = $this->extractLimitValue($context, $extracts);
+                $offset = $this->extractOffsetValue($context, $extracts);
+
+                if ($pagination instanceof OffsetPagination) {
+                    $pagination->offset = $offset;
+                    $pagination->limit = $limit;
+                }
+
                 if ($this->query) {
-                    $context = ($this->query)($query, $pagination, $context);
+                    $context = ($this->query)($query, $pagination, $context, $filters, $sort, $offset, $limit);
 
                     if (! $context instanceof Context) {
                         throw new RuntimeException('The Index endpoint query closure must return a Context instance.');
@@ -140,8 +145,8 @@ protected function setUp(): void
                     /** @var Context $context */
                     $context = $context->withQuery($query);
 
-                    $this->applySorts($query, $context);
-                    $this->applyFilters($query, $context);
+                    $this->applySorts($query, $context, $sort);
+                    $this->applyFilters($query, $context, $filters);
 
                     if ($pagination) {
                         $pagination->apply($query);
@@ -206,9 +211,9 @@ public function defaultSort(?string $defaultSort): static
         return $this;
     }
 
-    final protected function applySorts($query, Context $context): void
+    final protected function applySorts($query, Context $context, ?array $sort): void
     {
-        if (! ($sortString = $context->queryParam('sort', $this->defaultSort))) {
+        if (! $sort) {
             return;
         }
 
@@ -220,7 +225,7 @@ final protected function applySorts($query, Context $context): void
 
         $sorts = $collection->resolveSorts();
 
-        foreach (parse_sort_string($sortString) as [$name, $direction]) {
+        foreach ($sort as $name => $direction) {
             foreach ($sorts as $field) {
                 if ($field->name === $name && $field->isVisible($context)) {
                     $field->apply($query, $direction, $context);
@@ -234,18 +239,12 @@ final protected function applySorts($query, Context $context): void
         }
     }
 
-    final protected function applyFilters($query, Context $context): void
+    final protected function applyFilters($query, Context $context, array $filters): void
     {
-        if (! ($filters = $context->queryParam('filter'))) {
+        if (empty($filters)) {
             return;
         }
 
-        if (! is_array($filters)) {
-            throw (new BadRequestException('filter must be an array'))->setSource([
-                'parameter' => 'filter',
-            ]);
-        }
-
         $collection = $context->collection;
 
         if (! $collection instanceof Listable) {

diff --git a/framework/core/tests/integration/api/groups/ListTest.php b/framework/core/tests/integration/api/groups/ListTest.php
@@ -40,8 +40,10 @@ public function shows_limited_index_for_guest()
             $this->request('GET', '/api/groups')
         );
 
-        $this->assertEquals(200, $response->getStatusCode());
-        $data = json_decode($response->getBody()->getContents(), true);
+        $body = $response->getBody()->getContents();
+
+        $this->assertEquals(200, $response->getStatusCode(), $body);
+        $data = json_decode($body, true);
 
         // The four default groups created by the installer
         $this->assertEquals(['1', '2', '3', '4'], Arr::pluck($data['data'], 'id'));