rgh-netiquette - Support new views - #8141

I have tried bleow code and add some class for TimeLineItem, but it doesn't work.

export default function TimelineItem({
	className,
}: {
	className?: string;
}): JSX.Element {}

And it thorw an error: TypeError: Cannot read properties of undefined. If I add defaultProps, the className is concated not overrode, so it also doesn't work.

I also check the dom-chef lib, I don't find anything useful for me.

Unlike React components, any attributes you pass to dom-chef JSX will automatically be added. For classes, they will be appended. So <TimelineItem className='something'> will just work.

However you most likely just want to change the TimelineItem itself since we do want it to stay compatible across all the features that use it (e.g. closing-remarks)

Wherever possible, when you make changes to components or classes, search the codebase for the keyword and you might be able to fix multiple features at once. e.g. like you did for color-fg-muted in #7313

Unlike React components, any attributes you pass to dom-chef JSX will automatically be added. For classes, they will be appended. So will just work.

As you can see I add ml-md-0 to remove the margin, but it be overrode by ml-md-6. This is the reason why I want to have a customize classname here.

As you can see I add ml-md-0 to remove the margin, but it be overrode by ml-md-6

Since the component needs to work in both layouts, then the right class needs to be applied depending on context. TimelineItem is very simple so you can just duplicate it and export a new version just for the new timeline, e.g.

export function TimelineItemOld(): JSX.Element {
	// Classes copied from #issuecomment-new + mt-3 added
	return <div className="ml-0 pl-0 ml-md-6 pl-md-3 mt-3" />;
}

!! adjust/remove all classes as needed
export function TimelineItem(): JSX.Element {
	return <div className="ml-0 pl-0 ml-md-0 pl-md-3 mt-3" />;
}

And then in the features you can use:

const Wrapper = yourLogicToDetectOldView ? TimelineItemOld : TimelineItem;

const banner = (
	<Wrapper>{createBanner({etc})}</Wrapper>
)

You can alternatively use the createBanner style instead of JSX, like:

const banner = createTimelineItem(
	yourLogicToDetectOldView ? 'old' : 'react',
	createBanner({etc})
)

Up to you

Wait, to me it looks like it's in the wrong container. The banner should not go under the sidebar for example, it should be part of the main thread column

Wait, to me it looks like it's in the wrong container. The banner should not go under the sidebar for example, it should be part of the main thread column

No, it just on the front of comment box. But it is part of the comment container.