Skip to content
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

basic landing page #3470

Merged
merged 17 commits into from
May 4, 2023
Merged

basic landing page #3470

merged 17 commits into from
May 4, 2023

Conversation

ShaiahWren
Copy link
Contributor

@ShaiahWren ShaiahWren commented Mar 30, 2023

Issue: AAH-2172

This work is the first version of the Beta Galaxy landing page dashboard with footer. The design follows and adds to the mockups created by Anastasia Ratti.

The 'Cookie preferences' link on the footer (seen on redhat.com) is NOT included in this pr.

Screenshots

Body:
Screenshot 2023-04-24 at 11 15 05 AM

Footer:
Screenshot 2023-04-28 at 12 18 14 PM

@ShaiahWren ShaiahWren requested a review from himdel March 30, 2023 15:29
@github-actions github-actions bot added the backport-4.6 This PR should be backported to stable-4.6 (2.3) label Mar 30, 2023
@ShaiahWren ShaiahWren marked this pull request as draft March 30, 2023 15:29
@himdel himdel added community comunity mode feature and removed backport-4.6 This PR should be backported to stable-4.6 (2.3) labels Mar 30, 2023
@himdel
Copy link
Collaborator

himdel commented Mar 30, 2023

Thanks, yeah, I think this looks good 👍, enough to start discussions about what should be there :)

Still, when you have time, it might make sense to hide/remove the filter & pagination bits and add a section card with text, whether lorem ipsum or copied from https://galaxy.ansible.com/, because it's more likely we'll only have static content here now. (And maybe add a screenshot to the PR description :).)

Then, to get this ready for actually being the landing page .. it might or might not get its own menu item .. but clicking the top left logo would go to Paths.landingPage instead of Paths.collections (:heavy_check_mark:)

And it's still not a landing page if it's not the default page .. which is always the one with / for url in our case...
So, you'd then replace any use of Paths.search with Paths.collections (for all the links that should still be going to collections),
remove Paths.search, and change Paths.landingPage from /landing-page to /. (:heavy_check_mark:)

That should get us to the state where this is the landing page everywhere, if we need this to be different per mode,
either we have logic in the router assigning the / route to either the landing page component or the collections component, or the landing page component could navigate to a the right place based on logic in componentDidMount.

src/paths.ts Outdated Show resolved Hide resolved
@ShaiahWren ShaiahWren force-pushed the landing-page branch 2 times, most recently from 632b6f0 to 5587b0b Compare April 10, 2023 14:04
@ShaiahWren ShaiahWren requested a review from himdel April 10, 2023 14:05
@himdel
Copy link
Collaborator

himdel commented Apr 10, 2023

The changes look good 👍.

But the cards now take up all horizontal space instead of going side by side...
this should work..

--- a/src/components/cards/landing-page-card.tsx
+++ b/src/components/cards/landing-page-card.tsx
@@ -8,13 +8,14 @@ interface IProps {
 
 export const LandingPageCard = ({ title, body }: IProps) => {
   return (
-    <React.Fragment>
-      <Card className='landing-page-card' style={{ marginBottom: '20px' }}>
+    <Card
+      className='landing-page-card'
+      style={{ margin: '0 0 24px 24px', flex: '30%' }}
+    >
       <CardHeader>
         <CardTitle>{title}</CardTitle>
       </CardHeader>
       <CardBody>{body}</CardBody>
     </Card>
-    </React.Fragment>
   );
 };
--- a/src/containers/landing/landing-page.tsx
+++ b/src/containers/landing/landing-page.tsx
@@ -61,6 +35,14 @@ export class LandingPage extends React.Component<RouteProps, IState> {
         ></AlertList>
         <BaseHeader title={t`Home`} />
         <Main>
+          <div
+            style={{
+              display: 'flex',
+              flexWrap: 'wrap',
+              alignContent: 'flex-start',
+              marginLeft: '-24px',
+            }}
+          >
             <LandingPageCard
               title={t`Lorem Ipsum`}
               body={t`Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam nibh odio, semper non ex vitae, semper convallis tellus. Praesent et ipsum erat. Praesent hendrerit urna eget mattis vestibulum. Maecenas dictum orci vitae nisl sagittis laoreet id et mauris. Sed pharetra accumsan nibh a viverra. Duis tincidunt eros at maximus sodales. Fusce gravida tellus ligula eu posuere lorem placerat ut.`}
@@ -77,6 +59,7 @@ export class LandingPage extends React.Component<RouteProps, IState> {
               title={t`Lorem Ipsum`}
               body={t`Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam nibh odio, semper non ex vitae, semper convallis tellus. Praesent et ipsum erat. Praesent hendrerit urna eget mattis vestibulum. Maecenas dictum orci vitae nisl sagittis laoreet id et mauris. Sed pharetra accumsan nibh a viverra. Duis tincidunt eros at maximus sodales. Fusce gravida tellus ligula eu posuere lorem placerat ut.`}
             />
+          </div>
         </Main>
       </React.Fragment>
     );

20230410185502

And you may want to clean up the state logic in LandingPage a bit more .. alerts still make sense, but there would be no paging params, no loading/unauthorized state, no items, no inputText...

Oh and can you also change Paths.search to Path.collections in developer_guidelines.md? Just to keep the example up to date :)


After that, the only remaining thing will be adding a redirect to collections when not running in community mode, we'll need to add a feature flag to enable the landing page. (similar to ansible/galaxy_ng#1669)

@himdel
Copy link
Collaborator

himdel commented Apr 19, 2023

Looks great! :)

The only visual change I'd suggest is moving the "Welcome to Beta Galaxy" text to replace the "Home" title.

Second thing, localization.. most of it is 👍 , but we should not be splitting strings in the middle of sentences,
"Search page", "Use the", "ansible-galaxy", "to find content for your project..", ", the command line tool..." as 5 separate strings are hard to translate, this needs to be one string .. we have the Trans component (from lingui/macro) for that, <Trans>Use the <a href=...>Search page</a> to find....</Trans> should do just that :)

Otherwise I think this is ready except for the community vs standalone mode logic..
I think the final final landing page won't be tied to wisdom/lightspeed, but this one kind of is... so maybe all we need for that is...

diff --git a/src/containers/landing/landing-page.tsx b/src/containers/landing/landing-page.tsx
index e63ba73e..64ecb03c 100644
--- a/src/containers/landing/landing-page.tsx
+++ b/src/containers/landing/landing-page.tsx
@@ -10,10 +10,12 @@ import {
   closeAlertMixin,
 } from 'src/components';
 import { AppContext } from 'src/loaders/app-context';
+import { Paths, formatPath } from 'src/paths';
 import { RouteProps, withRouter } from 'src/utilities';
 
 interface IState {
   alerts: AlertType[];
+  redirect: boolean;
 }
 
 export class LandingPage extends React.Component<RouteProps, IState> {
@@ -22,11 +24,24 @@ export class LandingPage extends React.Component<RouteProps, IState> {
 
     this.state = {
       alerts: [],
+      redirect: false,
     };
   }
 
+  componentDidMount() {
+    const { ai_deny_index } = this.context.featureFlags;
+    if (!ai_deny_index) {
+      this.setState({ redirect: true });
+    }
+  }
+
   render() {
-    const { alerts } = this.state;
+    const { alerts, redirect } = this.state;
+
+    if (redirect) {
+      setTimeout(() => this.props.navigate(formatPath(Paths.collections)));
+      return null;
+    }
 
     return (
       <React.Fragment>

@ShaiahWren ShaiahWren force-pushed the landing-page branch 2 times, most recently from 5c5eec9 to c623da8 Compare April 24, 2023 17:54
@himdel
Copy link
Collaborator

himdel commented Apr 24, 2023

Translations look good now,
the redirect seems to work,
screenshot test failures are expected,
almost ready :)


Just the rediret above, and we need to fix the EE test failure ... looks like the redirect happens after the test clicks the EE menu item, but before it finishes loading the EE screen, so...

  1. test finishes login, loads the landing page
  2. test clicks the EE menu item
  3. redirect to collections triggers
  4. collections end up loading, not EE
  5. test tries to do EE stuff on collections page

I think this should help.. https://reactrouter.com/en/main/start/overview#pending-navigation-ui
only do the redirect when navigation.state !== "loading"
(but if it doesn't, adding a cy.wait before that one cy.menuGo will work (allowing it to load collections before clicking on EE))

@ShaiahWren ShaiahWren force-pushed the landing-page branch 3 times, most recently from 19aa52e to b5028a7 Compare April 25, 2023 18:02
<p>
<Trans>
Use the{' '}
<a href='https://galaxy.ansible.com/search?deprecated=false&keywords=&order_by=-relevance'>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, we'll need to update the Search page link to link to the new galaxy Search page, not the old one...

can you use a Link to Paths.collections here?

@ShaiahWren ShaiahWren marked this pull request as ready for review May 2, 2023 14:07
Issue: AAH-2172
Issue: AAH-2172
@jerabekjiri
Copy link
Contributor

I wonder if this would be a good idea to be a clickable link to landing page, but I guess not really since the logo is right above it
Screenshot from 2023-05-03 15-25-52

@jerabekjiri
Copy link
Contributor

jerabekjiri commented May 3, 2023

just small nitpicks, other than LGTM 👍 :)

@newswangerd newswangerd merged commit e926990 into ansible:master May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community comunity mode feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants