New homepage #5613

BSFishy · 2023-12-14T19:05:40Z

Description

Implements the new homepage design. The scope of this change is limited to the structure of the page and configuration through saved objects. Better configuration and more features will be added in subsequent PRs.

Issues Resolved

Closes #4966
Closes #5251

Screenshot

Testing the changes

Unit tests have been implemented for the section type API, to ensure that the service works as expected.

Check List

All tests pass
- yarn test:jest
- yarn test:jest_integration
New functionality includes testing.
New functionality has been documented.
Update CHANGELOG.md
Commits are signed per the DCO using --signoff

Signed-off-by: Matt Provost <[email protected]>

codecov · 2023-12-14T19:22:05Z

Codecov Report

Attention: 47 lines in your changes are missing coverage. Please review.

Comparison is base (058dfbc) 67.03% compared to head (9e0d5b3) 67.03%.
Report is 22 commits behind head on main.

Files	Patch %	Lines
.../home/public/services/section_type/section_type.ts	70.00%	12 Missing and 3 partials ⚠️
...on/components/homepage/sections/work_with_data.tsx	25.00%	9 Missing ⚠️
...tion/components/homepage/sections/learn_basics.tsx	30.00%	7 Missing ⚠️
src/plugins/home/public/plugin.ts	50.00%	4 Missing ⚠️
.../public/services/section_type/section_type.mock.ts	63.63%	4 Missing ⚠️
src/plugins/home/server/saved_objects/homepage.ts	25.00%	3 Missing ⚠️
...application/components/homepage/sections/utils.tsx	33.33%	2 Missing ⚠️
...ved_objects_management/public/register_services.ts	0.00%	1 Missing and 1 partial ⚠️
...gins/home/public/saved_homepage/_saved_homepage.ts	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5613      +/-   ##
==========================================
- Coverage   67.03%   67.03%   -0.01%     
==========================================
  Files        3296     3304       +8     
  Lines       63343    63456     +113     
  Branches    10087    10104      +17     
==========================================
+ Hits        42465    42536      +71     
- Misses      18429    18462      +33     
- Partials     2449     2458       +9

Flag	Coverage Δ
Linux_1	`35.16% <7.29%> (-0.07%)`	⬇️
Linux_2	`55.18% <ø> (ø)`
Linux_3	`43.83% <7.52%> (-0.10%)`	⬇️
Linux_4	`35.42% <58.77%> (+0.09%)`	⬆️
Windows_1	`35.19% <7.29%> (-0.07%)`	⬇️
Windows_2	`55.15% <ø> (ø)`
Windows_3	`43.83% <7.52%> (-0.10%)`	⬇️
Windows_4	`35.42% <58.77%> (+0.09%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Matt Provost <[email protected]>

src/plugins/home/public/application/components/homepage/section.tsx

src/plugins/home/public/application/components/home_app.js

src/plugins/home/server/capabilities_provider.ts

BSFishy · 2023-12-17T22:29:47Z

src/plugins/home/server/saved_objects/homepage.ts

+export const homepageSavedObjectType: SavedObjectsType = {
+  name: 'homepage',
+  hidden: false,
+  namespaceType: 'single',


Should probably look into if this is correct or if this value should be something different

Signed-off-by: Matt Provost <[email protected]>

src/plugins/home/public/application/components/homepage/section.tsx

src/plugins/home/public/assets/ingestion.png

Signed-off-by: Matt Provost <[email protected]>

src/plugins/home/public/services/section_type/section_type.ts

SuZhou-Joe · 2024-01-25T03:19:08Z

src/plugins/home/public/application/components/home_app.js

-              telemetry={telemetry}
-            />
-          </Route>
+          {homeConfig.disableNextHomePage ? (


If disabled, shall we register the new homepage? From the consume logic, it is more like isNextHomePageDefaultIndexPage.

It was implemented like this after a discussion with @AMoo-Miki. I'm open to different naming, but I think disableNextHomePage is less confusing. The majority of users are going to just go to the default route, so it effectively functions the same

Sure, just concern on if we should register the useless route.

The route is not useless, as it needs to be viewable when "off" so that an admin or user can preview it.

I see the confusion now. I've renamed it to something that better reflects its functionality.

Signed-off-by: Matt Provost <[email protected]>

AMoo-Miki · 2024-02-01T19:25:24Z

src/plugins/home/public/application/components/homepage/homepage.tsx

+  // TODO: is there a better way to check if the title is custom?
+  const title =
+    branding.applicationTitle !== 'OpenSearch Dashboards'


Surprisingly no. Different parts of the code consider the default title to be varying cases of this. Since this is what every other place is using, it should be safe enough.

ashwin-pc

I must say, for your first real contribution to OSD and considering the size of the PR and the number of not very well-documented features that you've touched here, this is a phenomenally well-written PR. All my comments are nitpicking stuff except for the navigation API change. I did not expect that. Great job! I even played around with the UI and found no issues no matter which way I pushed it. Did not see that coming.

ashwin-pc · 2024-02-05T09:38:01Z

src/plugins/home/public/application/components/homepage/_homepage.scss

@@ -0,0 +1,9 @@
+.home-homepage-pageBody {


nit:

Suggested change

.home-homepage-pageBody {

.homepagePage {

following BEM

ashwin-pc · 2024-02-06T01:26:41Z

src/plugins/home/public/application/components/homepage/footer.tsx

+      <RedirectAppLinks application={application}>
+        <EuiButtonEmpty
+          flush="both"
+          href={getUrlForApp('management', { path: 'opensearch-dashboards/settings#defaultRoute' })}


we need to do this for all links since they are internal links and this is how you trigger SPA routing

Suggested change

href={getUrlForApp('management', { path: 'opensearch-dashboards/settings#defaultRoute' })}

onClick={() => navigateToApp('management', { path: 'opensearch-dashboards/settings#defaultRoute' })}

ashwin-pc · 2024-02-06T01:27:06Z

src/plugins/home/public/application/components/homepage/footer.tsx

+  const getUrlForApp = application.getUrlForApp;
+  const { show, save } = application.capabilities.advancedSettings ?? {};


Suggested change

const getUrlForApp = application.getUrlForApp;

const { show, save } = application.capabilities.advancedSettings ?? {};

const { navigateToApp, capabilities } = application;

const { show, save } = capabilities.advancedSettings ?? {};

ashwin-pc · 2024-02-06T01:52:15Z

src/plugins/home/public/services/section_type/section_type.ts

+        error$.next(undefined);
+
+        // TODO: make this debounce time configurable
+        const combinedSave$ = combineLatest([heroes$, sections$]).pipe(debounceTime(1000));


nit:

Suggested change

const combinedSave$ = combineLatest([heroes$, sections$]).pipe(debounceTime(1000));

const combinedSections$ = combineLatest([heroes$, sections$]).pipe(debounceTime(1000));

ashwin-pc · 2024-02-06T01:54:04Z

src/plugins/home/public/services/section_type/section_type.ts

+    const subscriptions = new Subscription();
+
+    this.fetchHomepageData()
+      .then((homepage) => {


nit:

Suggested change

.then((homepage) => {

.then((homepageSavedObj) => {

ashwin-pc · 2024-02-06T02:01:55Z

src/plugins/home/public/saved_homepage/_saved_homepage.ts

+        id: (opts.id as string) || '',
+        defaults: {
+          heroes: [],
+          sections: [{ id: 'home:workWithData' }, { id: 'home:learnBasics' }],


Was wondering how you'd initialize it. Nice!

ashwin-pc · 2024-02-06T04:14:40Z

src/plugins/home/public/application/components/homepage/sections/work_with_data.tsx

+const Card: FC<{
+  imgSrc: string;
+  imgAlt: string;
+  title: string;
+  description: string;
+  footerButtonProps?: EuiButtonProps;
+  footerUrl: string;
+  footerText: string;
+  footerExternal?: boolean;
+}> = ({


nit:

Suggested change

const Card: FC<{

imgSrc: string;

imgAlt: string;

title: string;

description: string;

footerButtonProps?: EuiButtonProps;

footerUrl: string;

footerText: string;

footerExternal?: boolean;

}> = ({

interface CardProps {

imgSrc: string;

imgAlt: string;

title: string;

description: string;

footer: {

buttonProps?: EuiButtonProps;

url: string;

text: string;

external?: boolean;

};

}

const Card: FC<CardProps> = ({

ashwin-pc · 2024-02-06T04:16:04Z

src/plugins/home/public/application/components/homepage/sections/utils.tsx

Nice, i like this pattern. Nit, could use some tests

ashwin-pc · 2024-02-06T04:21:25Z

src/plugins/home/public/application/components/homepage/homepage.tsx

+  const sideItems: React.ReactNode[] = [
+    <EuiButtonEmpty
+      iconType="wrench"
+      href={getUrlForApp('dev_tools', { path: '#/console' })}


Same here, use navigateToApp

@ashwin-pc, to expedite this, can you please make the suggestion and then accept it?

abbyhu2000 · 2024-03-05T21:09:22Z

src/plugins/home/public/services/section_type/section_type.ts

+    const homepage: SavedHomepage = await this.savedHomepageLoader.get(id);
+
+    if (!id) {
+      await homepage.save({});


What if save() fail here?

in getHomepage (The function that eventually calls this) he's called out about how errors need to be tested. I dont think he got to that in this iteration. I also see that he's throwing all the errors which mean that its up to the parent function to catch it and handle them. Dont think they are being handled here.

abbyhu2000 · 2024-03-05T21:34:30Z

src/plugins/home/public/application/components/homepage/homepage.tsx

+    const homepage = sectionTypes.getHomepage();
+
+    const subscriptions = new Subscription();
+    subscriptions.add(homepage.heroes$.subscribe(setHeroes));


i am confused on why we need to add subscriptions on sections and heroes here? Are these built for the functionalities of allowing users to add and delete sections or heroes in the future?

Because currently I see the implementation exposes the registerSection service API to allow other plugins to register sections or heroes, and it seems like for rendering something like the homepage, we can just fetch all the registered sections and heroes at the start of the application, and render the page all at once without the need of constant listening on them?

Yes the goal is to allow the user to modify these sections and customize the homepage. This PR does not have the editing capability just yet, but it does lay down the plumbing.

abbyhu2000 · 2024-03-05T21:43:03Z

src/plugins/home/public/saved_homepage/_saved_homepage.ts

+  sections: SerializedSection[] | SerializedSection;
+}
+
+export function createSavedHomepageClass(services: SavedObjectOpenSearchDashboardsServices) {


I am wondering why we need to introduce the saved object logic for implementing new homepage.

It seems like currently the concept of saved object within dashboard does not really associate with an entire page. And it also makes the homepage harder to maintain because of the possible saved object migrations.

I am wondering

why can't we use a simpler route such as utilizing the configuiSettings for saving homepage instead of creating a new saved object such as below; and the _id here also stores the version so we do not need to worry about versioning.

{ "_index": ".kibana_1", "_id": "config:2.12.0", "_score": 1, "_source": { "config": { "buildNum": 221231, "defaultIndex": "ff959d40-b880-11e8-a6d9-e546fe2bba5f" }, "type": "config", "references": [], "migrationVersion": { "config": "7.9.0" }, "updated_at": "2024-02-22T02:15:31.504Z" } }

or why cant we save user specific configs using browser storage as the source of truth that is referenced in this PR Feat (core): Make theme settings user-specific and user-configurable #5652?

I dont think i understood your comment about uiSettings. How does that work? How does that differ from Saved Objects?

Also Isnt the homepage just a glorified Dashboard? And that uses saved objects to figure out which embeddable to show and where. The homepage saved object does something similar too. Yes there will be a migration, but that will be needed wherever the user needs to be able to configure their own data.

As for briwser storage. The homepage needs to be tied to a user or workspace. It has to follow them no matter where they login. The PR you reference saves that in local storage which cannot do that

i think at first i did not understand why we need to introduce the saved object concept into homepage, i thought it is just a page that needs to be rendered once when starting up the application. And then if we needed to save some user configuration, maybe we can just do something like uiSettings.set("useHomePage", true), and if we need to see which homepage users want to use, we do uiSettings.get("useHomePage")

So after looking at your explanations, it seems like we want to make the homepage more configurable to the user, and almost similar to the concept of a dashboard, where user can create their own homepage, add or remove different sections to it, and save it, and that is why we want to use saved object, correct? @ashwin-pc

abbyhu2000 · 2024-03-05T23:05:09Z

src/plugins/home/public/services/section_type/section_type.ts

+}
+
+export interface Homepage {
+  heroes$: Observable<HeroSection[] | undefined>;


For the hero, why would happen if multiple different plugin register hero for themselves, which hero should we display? In my understanding, we should just display one hero per screen?

I dont think that was a requirement. Maybe based on how the homepage evolves we may restrict that. cc @kgcreative ?

abbyhu2000 · 2024-03-06T00:06:52Z

@ashwin-pc Could you also link me the issue for adding a toggle in advanced setting for the new home page?

Currently, i see we can toggle this feature in the yml file; do we wanna switch the toggle to advance setting toggle, or do we wanna keep both toggles? If we keep both, what if user toggle differently for two toggles?

ashwin-pc · 2024-03-18T22:38:43Z

closing the PR since #6065 has been merged. Thats the same PR, just that the toggle has moved from the yaml config to the advanced settings.

Layout initial new homepage implementation

0e4a30d

Signed-off-by: Matt Provost <[email protected]>

github-actions bot added the seasoned-contributor label Dec 14, 2023

BSFishy added 2 commits December 14, 2023 19:08

Add some things I for some reason didn't get in the initial commit

1572cac

Signed-off-by: Matt Provost <[email protected]>

Add formatting line

7babcc1

Signed-off-by: Matt Provost <[email protected]>

BSFishy added 14 commits December 14, 2023 20:38

Flesh out some more of the homepage implementation

8a48a5f

Signed-off-by: Matt Provost <[email protected]>

Add doc comment explaining get homepage function

f9183d4

Signed-off-by: Matt Provost <[email protected]>

Remove unused function

ca0eb55

Signed-off-by: Matt Provost <[email protected]>

Rename functions to more accurately describe what they do

76ec36a

Signed-off-by: Matt Provost <[email protected]>

WE HAVE RENDERING

1c00578

Signed-off-by: Matt Provost <[email protected]>

Make some fixed/clarifications around the lazy rendering

c4eea91

Signed-off-by: Matt Provost <[email protected]>

Add basic layout for hero sections and sections

f87bb41

Signed-off-by: Matt Provost <[email protected]>

Set up proper section rendering

cb4538c

Signed-off-by: Matt Provost <[email protected]>

Add page layout

35ffb80

Signed-off-by: Matt Provost <[email protected]>

Add homepage saved object edit button

6a8ae77

Signed-off-by: Matt Provost <[email protected]>

Memoize section content

ea5b7e1

Signed-off-by: Matt Provost <[email protected]>

Add data section

f50aa73

Signed-off-by: Matt Provost <[email protected]>

Add learn basics section

c0203d4

Signed-off-by: Matt Provost <[email protected]>

Small cleanup things

e192c1a

Signed-off-by: Matt Provost <[email protected]>

BSFishy commented Dec 19, 2023

View reviewed changes

BSFishy added 10 commits December 21, 2023 18:37

Misc changes

06a18f3

Signed-off-by: Matt Provost <[email protected]>

Remove custom card display

1d45b7c

Signed-off-by: Matt Provost <[email protected]>

Fix breadcrumbs not updating

cfe4512

Signed-off-by: Matt Provost <[email protected]>

Add small todo comment

2fc43e4

Signed-off-by: Matt Provost <[email protected]>

Inline section rendering

afedf5b

Signed-off-by: Matt Provost <[email protected]>

Fix section type service mock

ad7e3bd

Signed-off-by: Matt Provost <[email protected]>

Fix jest tests

9892fa3

Signed-off-by: Matt Provost <[email protected]>

Merge branch 'main' into newest-home

950f892

Fix the test mocks

f66827e

Signed-off-by: Matt Provost <[email protected]>

Add throw tests

c60d61e

Signed-off-by: Matt Provost <[email protected]>