O layout tsx #898

akomiqaia · 2022-11-24T15:48:45Z

No description provided.

notlee

This came together more quickly than I expected, nice work 😄 I think there are areas we can improve though from a user experience perspective, as noted ~~above~~ in the review comments.

We should also review the controls to make sure they are all helpful. A lot of props are for children, which display this kind of nonsense in the controls! Perhaps some controls should be hidden. Maybe new controls should be configured within a story, as a more helpful demo for generating html to copy, and mapped to the relevant props.

notlee · 2022-12-01T16:13:57Z

components/o-layout/src/tsx/layout.tsx

+	hero?: HeroProps;
+	sidebar?: SideBarProps;
+	footer: ChildrenType;
+	displayArticleList?: boolean;


This prop toggles a o-layout-typography class right? Could you explain why it's called displayArticleList?

Aside, perhaps we should also off some o-layout typography wrapper tsx component o-layout-typography?

@notlee it refers to article list part of the documentation.

The o-layout-typography class can be used within any layout though, should the prop be more generic? FYI it can also can used multiple times throughout the page https://registry.origami.ft.com/components/o-layout@5.2.4/readme?brand=core#typography

So yea this toggles o-layout-typography for landing layout. I changed the name of it but according to docs if we are using article list inside the main content we need to remove o-layout-typography class.

components/o-layout/src/tsx/layout.tsx

components/o-layout/stories/internal/internalLayout.stories.tsx

notlee · 2022-12-01T16:23:05Z

components/o-layout/stories/internal/internalLayout.stories.tsx

+	return <QueryLayout {...args} />;
+};
+
+QueryLayoutDemo.storyName = 'Query Layout';


Navigation has been added in the query layout? I don't think that's intended?

@notlee documentation suggests that it supports sidebar navigation. Happy to update documentation and templates if this is no longer supported.

Oh cool, good spot, that's fine :) Though it's disabled by default so it would be better to be off by default in the demos too

a87d9e1 made a few changes here so we could destroy sidebar nav correctly and also not to allow duplicate nodes in case of rerendering

notlee · 2022-12-01T16:43:49Z

components/o-layout/src/tsx/landingPartials.tsx

+
+export function OverviewSections({
+	sections,
+	hasAction,


We don't need a user to specify a hasAction prop, we can derive it based on whether any sections have an actionElement

notlee · 2022-12-01T16:45:38Z

components/o-layout/src/tsx/landingPartials.tsx

@@ -0,0 +1,54 @@
+import {HeroProps, OverviewSectionsProps, ArticleListProps} from './layout';
+
+export function Hero({muted, heroContent}: HeroProps) {


Have you considered passing heroContent as children?

<Hero> <p>hello world!</p> </Hero>

vs.

const heroContent = <p>hello world!</p>; <Hero heroContent={heroContent}/>

notlee · 2022-12-01T16:47:52Z

components/o-layout/src/tsx/layout.tsx

+
+type SideBarProps = {
+	customNavHeadingSelector?: string;
+	customNavigation?: JSX.Element | JSX.Element[];


Similar to before, could customNavigation work well as children?

617c3a4 what do you think about this update?

A short summary of the commit:
removed SideBarProps and instead moved customNavHeadingSelector prop to controls. For custom navigation, if users pass down any children to DocsLayout it will render custom navigation. I added some comments and an example inside a story file.

or I guess we could use a decorator for this too and have a boolean control to display custom sidebar navigation?

notlee · 2022-12-01T16:50:33Z

components/o-layout/src/tsx/layout.tsx

+};
+
+type OverviewSectionItemProps = {
+	element: ChildrenType;


I don't think element is very descriptive here, perhaps content? Although again maybe children would be a better interface to provide section content?

00cac54 updated it to content. We could use children too but OverviewSections expects an array of objects.

notlee · 2022-12-01T16:51:54Z

components/o-layout/src/tsx/layout.tsx

+
+type OverviewSectionItemProps = {
+	element: ChildrenType;
+	actionElement?: ChildrenType;


actionElement on the other hand sounds reasonably descriptive 👍

…text for our HTML plugin

…ies that had hero background on full width

akomiqaia · 2022-12-23T17:49:55Z

From the last discussion, we spoke about improving this PR. Changes discussed were:

Hide controls that are not in use, remove hero controls (e081ce7)
Storybook demos with hero layout had the full-width grey background, that comes from this style definition. At the moment stories that use hero are rendered at the fullscreen layout (padded by default) (e081ce7)
rename and then remove add styles control (09c2034)

akomiqaia added 2 commits November 23, 2022 15:48

backstage: inital tsx template for o-layout

0f0be57

backstage: TSX template for o-layout

14dda09

akomiqaia requested a review from a team as a code owner November 24, 2022 15:48

akomiqaia mentioned this pull request Nov 24, 2022

Recreate component / library demos in Storybook #522

Open

43 tasks

notlee temporarily deployed to origami-webs-o-layout-t-dcxirq November 24, 2022 15:53 Inactive

Merge branch 'main' into o-layout-tsx

1e3bbae

akomiqaia mentioned this pull request Nov 29, 2022

Stories/o layout #358

Closed

notlee temporarily deployed to origami-webs-o-layout-t-vpgdjb November 29, 2022 15:47 Inactive

docs: o-layout internal storybook syntax error

2a01ec1

notlee temporarily deployed to origami-webs-o-layout-t-wa52df December 1, 2022 16:46 Inactive

notlee requested changes Dec 1, 2022

View reviewed changes

Merge branch 'main' into o-layout-tsx

597a1df

notlee temporarily deployed to origami-webs-o-layout-t-ogpjlg December 6, 2022 11:09 Inactive

akomiqaia and others added 15 commits December 8, 2022 15:51

Merge branch 'main' into o-layout-tsx

39c1432

Merge branch 'main' into o-layout-tsx

22392a6

backstage: import header as a story and publish tsx template to NPM

b28cd7f

backstage: rewrite o-layout tsx and import different layouts separetly

378d8db

backstage: add decorator for header-services so we dont output dummy …

2303c1e

…text for our HTML plugin

backstage: write new types for different types of layouts

c3f3452

backstage: update docLayout markup and simplify component API

617c3a4

backstage: change sidebar to constructNav for more clarity

3138b4e

backstage: update sidebar navigation support for different layers

a87d9e1

backstage: update hero demos with articleList and overviewSection

27c5d07

backstage: reverse fancy controls and disable them

ac14922

backstage: simplify Overview sections API

e25ba6f

backstage: update OverviewSections API for clarity

00cac54

Merge branch 'main' into o-layout-tsx

1e5c9a7

backstage: remove unused controls and add layout: fullscreen for stor…

e081ce7

…ies that had hero background on full width

backstage: update layout prop to more clear prop name

09c2034

Merge branch 'main' into o-layout-tsx

ff25939

notlee self-requested a review January 4, 2023 08:42

feat: publish o-layout tsx

f6c70a6

notlee approved these changes Jan 4, 2023

View reviewed changes

notlee enabled auto-merge (squash) January 4, 2023 08:44

notlee merged commit 862300f into main Jan 4, 2023

notlee deleted the o-layout-tsx branch January 4, 2023 08:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

O layout tsx #898

O layout tsx #898

akomiqaia commented Nov 24, 2022

notlee left a comment •

edited

Loading

notlee Dec 1, 2022

akomiqaia Dec 6, 2022

notlee Dec 6, 2022

akomiqaia Dec 20, 2022

notlee Dec 1, 2022

akomiqaia Dec 6, 2022

notlee Dec 6, 2022

akomiqaia Dec 19, 2022

notlee Dec 1, 2022

akomiqaia Dec 20, 2022

notlee Dec 1, 2022

notlee Dec 1, 2022

akomiqaia Dec 16, 2022

akomiqaia Dec 16, 2022

notlee Dec 1, 2022

akomiqaia Dec 20, 2022

notlee Dec 1, 2022

akomiqaia commented Dec 23, 2022

		@@ -0,0 +1,54 @@
		import {HeroProps, OverviewSectionsProps, ArticleListProps} from './layout';

		export function Hero({muted, heroContent}: HeroProps) {

O layout tsx #898

O layout tsx #898

Conversation

akomiqaia commented Nov 24, 2022

notlee left a comment • edited Loading

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akomiqaia commented Dec 23, 2022

notlee left a comment •

edited

Loading