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.

Sign up for GitHub

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

Fix related pubs bug #1017

Merged
merged 3 commits into from
Mar 4, 2025
Merged

Fix related pubs bug #1017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6e726e6
Remove prose class which added unwanted styling
allisonking Mar 4, 2025
0711707
Concat related pubs to pub context
allisonking Mar 4, 2025
ac9dbe6
Merge branch 'main' into aking/1003/related-pubs-bug
allisonking Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/app/c/[communitySlug]/pubs/[pubId]/edit/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export default async function Page(props: {
}
>
<div className="flex justify-center py-10">
<div className="prose max-w-prose flex-1">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this originally came from #980 but it seems like just the flex-1 is enough to keep the width. adding prose changes a bunch of styles, including making <hr/>s have huge margins

image

prose is used for markdown, but I believe the various markdown places already have prose on them, i.e. StructuralElement

platform/core/app/components/FormBuilder/FormElement.tsx

Line 167 in 0711707

<div className={cn("prose prose-sm", element.deleted ? "text-gray-500" : "")}>

<div className="max-w-prose flex-1">
<PubEditor
searchParams={searchParams}
pubId={pub.id}
Expand Down
2 changes: 1 addition & 1 deletion core/app/c/[communitySlug]/pubs/create/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export default async function Page(props: {
right={<div />}
>
<div className="flex justify-center py-10">
<div className="prose max-w-prose flex-1">
<div className="max-w-prose flex-1">
<PubEditor
searchParams={searchParams}
communityId={community.id}
Expand Down
16 changes: 12 additions & 4 deletions core/app/components/pubs/PubEditor/PubEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { randomUUID } from "crypto";

import type { ProcessedPub } from "contracts";
import type { CommunitiesId, PubFieldsId, PubsId, PubTypesId, StagesId } from "db/public";
import type { CommunitiesId, PubsId, PubTypesId, StagesId } from "db/public";
import { CoreSchemaType } from "db/public";
import { expect } from "utils";

import type { FormElements, PubFieldElement } from "../../forms/types";
Expand Down Expand Up @@ -44,8 +45,10 @@ const RelatedPubValueElement = ({
<p className="text-sm">
You are creating a Pub related to{" "}
<span className="font-semibold">{getPubTitle(relatedPub)}</span> through the{" "}
<span className="font-semibold">{fieldName}</span> pub field. Please enter a
value for this relationship.
<span className="font-semibold">{fieldName}</span> pub field.
{element.schemaName !== CoreSchemaType.Null && (
<span>Please enter a value for this relationship.</span>
)}
Comment on lines +49 to +51
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just noticed it was still saying to fill out a field if the schema was null (and then it wouldn't render an input to fill out)

</p>
<PubFieldFormElement
label={label}
Expand Down Expand Up @@ -269,12 +272,17 @@ export async function PubEditor(props: PubEditorProps) {
const currentStageId = pub?.stage?.id ?? ("stageId" in props ? props.stageId : undefined);
const pubForForm = pub ?? { id: pubId, values: [], pubTypeId: form.pubTypeId };

// For the Context, we want both the pubs from the initial pub query (which is limited)
// as well as the pubs related to this pub
const relatedPubs = pub ? pub.values.flatMap((v) => (v.relatedPub ? [v.relatedPub] : [])) : [];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL: you can use flatMap to get around how doing a .map(...).filter(value => !!value) doesn't let TS think you have Thing[] and TS will still think you have (Thing | null)[] even after the filter

https://stackoverflow.com/a/59726888

Copy link
Member

Choose a reason for hiding this comment

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

very hot tip, the .filter typing is so annoying!

Copy link
Member

Choose a reason for hiding this comment

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

i think they did fix this in a recent release, if you do .filter(value=>value != null) specifically!

Copy link
Member

Choose a reason for hiding this comment

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

oh no nvm, this only applies one level deep! they fixed the "simple" case in typescript 5.5

https://devblogs.microsoft.com/typescript/announcing-typescript-5-5/#inferred-type-predicates

Example (change to typescript <5.5 to see error in the simple case)

https://www.typescriptlang.org/play/?target=99&pretty=true&allowSyntheticDefaultImports=true&preserveValueImports=true&isolatedModules=true&verbatimModuleSyntax=false&ts=5.5.4&ssl=1&ssc=1&pln=2&pc=1#code/C4TwDgpgBAwg9gWzAGwgDwCoAsCWA7AcygF4oBvKLCEALnMrjoHIrlk4omBfKAHyjwBXNlwBQoSFADKOJKmz4ipClVqdW7JlB78hbUQBMIAY2QBDAE7RjcPAGdgUG3PR0AFPBeZchAJQBtAF1DE3MrJ1sHKDtZFAg6GRcFQiDRURt7R2cwEgivADoAMxxkYAgLNwA3YgA+SvzVKABCUj1kX1FURys7XOz-AAZAhuoGjlEAegmoGagAPQB+KDSMqJikXPW4opKyiuqapqbKjq6oHoAmTdjB4dVJ6dnFqCA

const pubsForContext = [...pubs, ...relatedPubs];

return (
<FormElementToggleProvider fieldSlugs={allSlugs}>
<ContextEditorContextProvider
pubId={pubId}
pubTypeId={pubType.id}
pubs={pubs}
pubs={pubsForContext}
pubTypes={pubTypes}
>
<PubEditorWrapper
Expand Down
Loading