Add news announcement carousel to dashboard #9617

cl8n · 2022-12-09T14:03:02Z

changes from the design on figma:

no rounded corners, because rest of this page doesn't have them either
height is slightly different, because the news post previews also have a thinner layout than on figma
added the markdown overlay

i wasn't sure how it was supposed to animate so I made it slide horizontal. if you aren't interacting with it, it'll automatically rotate every 6 seconds (arbitrary), and otherwise you can control it yourself with the buttons on the side

news_announcements is also intended to be the lazer version of stable's main menu banners, but for now no endpoint or anything.

resolves #9614

nanaya · 2022-12-14T10:19:02Z

app/Models/NewsAnnouncement.php

+ * @property string $url
+ * @method static Builder default()
+ */
+class NewsAnnouncement extends Model


It's not always about News. And may not even about Announcement.

i recommend something in the vein of "highlights" as per the original designs

"Highlight" sounds fine to me but it's kind of a generic word, just in this project it's already used a lot regarding search or styling
DashboardHighlight? Headline? MainMenuBanner? idk what to call this thing lol

just Banner is probably fine

resources/views/home/user.blade.php

resources/assets/lib/news-announcements/main.tsx

nanaya · 2022-12-14T11:12:15Z

resources/assets/lib/news-announcements/main.tsx

+      return (
+        <div className={`${bn}__indicators`}>
+          {this.props.announcements.map((_, index) => (
+            <div


these should probably be clickable

they're so small (6px max height) that it's kind of annoying to try to click them. I can't tell from the design if they were intended to be buttons...

I don't think it's too bad as long the button expands on hover

added -- I was thinking that if the button still feels too thin, the hoverable/clickable height could be increased while keeping the display as-is. but I'll wait for your input

resources/assets/lib/news-announcements/main.tsx

app/Models/NewsAnnouncement.php

resources/assets/lib/news-announcements/main.tsx

nanaya · 2023-01-04T11:28:30Z

I just remembered, should this be shared with in-client banner system? The actual image will be different but I imagine there should be some overlaps in the content itself (if not exactly the same).

Walavouchey · 2023-01-04T15:00:46Z

the client banner can only display one at a time (but i do believe i've heard they can be set to automatically rotate) while this website carousel can display multiple. the intention with the linked issue was that they would have the same type of content, yes (and as mentioned in op, eventually displayed in lazer too)

nanaya · 2023-01-06T08:16:14Z

So the client banner currently isn't stored anywhere. The new table here should add client_image_url so bancho can be updated to use it as well.

Hide overflow on the top element rather than the announcements container, so now the container can be transformed by CSS rather than scrolled through. And clean up many other things that resulted from not needing to track element widths and such.

And add transition to height

cl8n · 2023-08-09T04:17:42Z

okay, a lot of updates here. the main react component is basically rewritten now to use only CSS for the positions and animations, which resolved a lot of your reviews just by being able to get rid of all that ref/observer/etc stuff (ee2dd65)

I also made it support that "infinite scrolling" feel since it wasn't actually very difficult after rewriting it (tl;dr clone the announcements as needed to make it appear as if you can scroll to one side forever; delete unnecessary clones when it stops animating) (dfd92c8)

I just remembered, should this be shared with in-client banner system? The actual image will be different but I imagine there should be some overlaps in the content itself (if not exactly the same).

yep that's the goal as mentioned in OP, it should be able to work for web, stable, and also lazer whenever it gets a similar feature. just website for this PR though. once this PR is in, I'll get in touch w/ ephemeral or whoever is managing the main menu banners currently, to figure out how this system could be shared best

So it won't be confusing naming when there are multiple kinds of buttons

nanaya · 2023-08-15T12:58:36Z

database/migrations/2023_08_07_100000_create_news_announcements.php

+            $table->text('content_markdown')->nullable();
+            $table->timestamp('ended_at')->nullable();
+            $table->string('image_url');
+            $table->tinyInteger('order');
+            $table->timestamp('started_at')->useCurrent();
+            $table->string('url');


this ended up rather weird when viewed from a db client. At least the start/end time should be close together

nanaya · 2023-08-15T12:59:38Z

app/Models/NewsAnnouncement.php

+ * @property string $url
+ * @method static Builder default()
+ */
+class NewsAnnouncement extends Model


just Banner is probably fine

nanaya · 2023-08-15T13:05:43Z

resources/js/components/news-announcements.tsx

+import * as React from 'react';
+import { classWithModifiers } from 'utils/css';
+
+function modulo(dividend: number, divisor: number): number {


this is more like positiveModulo

nanaya · 2023-08-15T13:25:09Z

resources/css/bem/news-announcement.less

+    height: 100%;
+    left: 50%;
+    position: absolute;
+    transform: translateX(-50%);


This way probably handles fitting in the box better (including when the image is too wide)

height: 100%; width: 100%; object-fit: contain; // or cover

nanaya · 2023-08-15T13:46:40Z

resources/css/bem/user-home.less

+    height: 250px; // news-announcements__container height
+    justify-content: center;
+    margin-top: 20px; // news-announcements margin
+
+    &--with-indicators {
+      // This part won't be identical to the real component if the indicators
+      // wrap onto more than one line, but it's good enough for a placeholder
+      margin-bottom:
+        10px /* news-announcements__indicators margin */ +
+        6px /* news-announcements__indicator height */;


put the dimensions in variable maybe (or use the actual classes directly instead of copying style)

nanaya · 2023-08-15T13:53:50Z

resources/js/components/news-announcements.tsx

+    this.minIndex = 0;
+  };
+
+  private renderAnnouncement = (announcement: NewsAnnouncementJson, index = 0) => (


this looks like could be its own component

nanaya · 2023-08-15T13:59:55Z

resources/js/components/news-announcements.tsx

+    );
+  }
+
+  private clearAutoRotateTimer = () => {


class methods should be readonly

nanaya · 2023-08-15T14:09:51Z

resources/css/bem/news-announcement.less

+.news-announcement {
+  height: 100%;
+  left: calc(100% * var(--index, 0));
+  overflow-x: hidden;


what's this for? Doesn't the news-announcements__container already cover the overflow hiding?

nanaya · 2023-08-25T13:31:42Z

resources/js/components/news-announcements.tsx

+          {/*
+            Render the announcements, including clones before and after to help
+            create the illusion of an infinitely scrolling container
+          */}


where's the clone? the range below only generates exactly the length of announcements.

From what I can tell the illusion currently happens because the clone is generated exactly before the animation starts?

(oh and unless you go furious with clicking, index - 2, index + 3 range also works)

nanaya · 2024-05-08T09:52:35Z

@cl8n still interested in finishing this? In addition to the comments above, the data source should be changed to use this json file instead (env configurable). Can be either downloaded server side and rendered accordingly or done completely on front end.

cl8n · 2024-05-20T06:31:43Z

@nanaya there was some discussion about this that amounted to nobody being sure if this should share the same data as the in-game banners, because those often link to news on the front page, and it would be kind of redundant. ppy/osu#27680 (comment) (and then more on discord but there was no real conclusion)

personally I don't find sharing data to be problematic as long as whoever controls it is mindful about not literally duplicating content from news banners. but whoever those ppl are should probably be the ones deciding how this works

I'll respond to the previous review in the meantime

nanaya · 2024-05-20T09:38:06Z

if there's something that's really redundant (current top news post, and no other rotating banner) then can add flag indicating such in the json maybe.

Even duplicating is probably not as much problem as it's two different sections (current highlight vs news post listing). And if there's a new news post coming the older news post banner can still be the highlight.

cl8n added 3 commits December 9, 2022 05:52

Add news announcement model and transformer

d53a04e

Add news announcement carousel thing to dashboard

b253dd2

...JSON

a8259cb

nanaya requested changes Dec 14, 2022

View reviewed changes

notbakaneko requested changes Dec 14, 2022

View reviewed changes

cl8n added 14 commits August 6, 2023 22:04

Merge master

41aec09

Fix component and migration filename

e27be8b

Darker background for announcement content overlay

f8c965c

Make buttons cover full news announcement height

aba24ec

Null entire content JSON field if content_markdown is null

499abb2

Fix indicator rows changing size

fddcc3f

And add transition to height

Add "infinite scrolling" look to carousel

dfd92c8

Simpler auto rotate timer clear

77f8be2

Match indicator opacity to figma design

6e7403f

Add placeholder for news announcements to prevent page jump

f84a71a

Fix operator precedence in news announcement query

6c4f99d

Move news announcements component to component directory

d263666

Missing type

11be4fb

cl8n added 4 commits August 15, 2023 18:02

Rename "button" to "arrow"

9fa75db

So it won't be confusing naming when there are multiple kinds of buttons

Always return early from setIndex if index is the same

1b2067c

Move indicators inside the main div

ec6c83e

Make indicators act as buttons

3bc3816

nanaya requested changes Aug 25, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add news announcement carousel to dashboard #9617

Add news announcement carousel to dashboard #9617

cl8n commented Dec 9, 2022

nanaya Dec 14, 2022

Walavouchey Dec 14, 2022

cl8n Aug 9, 2023

nanaya Aug 15, 2023

nanaya Dec 14, 2022

cl8n Aug 9, 2023

nanaya Aug 15, 2023

cl8n Aug 16, 2023

nanaya commented Jan 4, 2023

Walavouchey commented Jan 4, 2023

nanaya commented Jan 6, 2023

cl8n commented Aug 9, 2023

nanaya Aug 15, 2023 •

edited

nanaya Aug 15, 2023

nanaya Aug 15, 2023

nanaya Aug 15, 2023

nanaya Aug 15, 2023

nanaya Aug 15, 2023

nanaya Aug 15, 2023

nanaya Aug 15, 2023

nanaya Aug 25, 2023 •

edited

nanaya commented May 8, 2024

cl8n commented May 20, 2024

nanaya commented May 20, 2024

Add news announcement carousel to dashboard #9617

Are you sure you want to change the base?

Add news announcement carousel to dashboard #9617

Conversation

cl8n commented Dec 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nanaya commented Jan 4, 2023

Walavouchey commented Jan 4, 2023

nanaya commented Jan 6, 2023

cl8n commented Aug 9, 2023

nanaya Aug 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nanaya Aug 25, 2023 • edited

Choose a reason for hiding this comment

nanaya commented May 8, 2024

cl8n commented May 20, 2024

nanaya commented May 20, 2024

nanaya Aug 15, 2023 •

edited

nanaya Aug 25, 2023 •

edited