Skip to content

#474 Add properly scaled minHeight, maxHeight, to webflow sync#568

Open
burnhamrobertp wants to merge 4 commits intobeyond-all-reason:mainfrom
burnhamrobertp:feature/474-sync-min-max-height
Open

#474 Add properly scaled minHeight, maxHeight, to webflow sync#568
burnhamrobertp wants to merge 4 commits intobeyond-all-reason:mainfrom
burnhamrobertp:feature/474-sync-min-max-height

Conversation

@burnhamrobertp
Copy link
Copy Markdown

These properties are already part of the metadata extracted and generated in the maps-cache metadata.json for each map. The values they have seem to use the same scale as mapWidth or mapHeight, and so are scaled accordingly before inclusion in the sync (value / 64)

…bflow sync

These properties are already part of the metadata extracted and generated in the maps-cache metadata.json for each map
Copy link
Copy Markdown
Collaborator

@p2004a p2004a left a comment

Choose a reason for hiding this comment

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

Modifying WebsiteMapInfo is not sufficient, as comment says "internal representation of data used in this script". To sync out to webflow it is converted back and forth with WebflowMapInfo that builds from the webflow_types that you should be able to refresh with the read token you have.

@burnhamrobertp
Copy link
Copy Markdown
Author

burnhamrobertp commented Feb 18, 2026

My utilization of makefiles has been pretty minimal since college. Running this repo in docker, with everything configured, with a .env file that has WEBFLOW_API_TOKEN and WEBFLOW_COLLECTION_ID I can echo $WEBFLOW_COLLECTION_ID or echo $WEBFLOW_API_TOKEN and get the expected value...however these values aren't present when running make refresh_webflow_types

What is the mechanism by which this is supposed to work, because I'm not seeing it.

Hrm, just setting EXPORT WEBFLOW_COLLECTION_ID=... seems to work; I wonder why the readme's setup isn't working though

@p2004a
Copy link
Copy Markdown
Collaborator

p2004a commented Feb 18, 2026

Have you sourced .env inside of container after you put export WEB... in it?

EDIT: forgot how it works, it is sourced automatically by container

&& echo "source .envrc" >> $HOME/.bashrc
I will assume your env file looked like

VAR=asdasd

instead of

export BAR=asdasd

and that's why variables were not available in script

Or you didn't rebuild container.

EDIT2: probably makes sense to change code to not require export in .env file. That's likely expected by people.

@burnhamrobertp
Copy link
Copy Markdown
Author

That's probably it, yes. I'm used to .env files that use VAR=asdf. Isn't export VAR=asdf bash-specific? I thought the whole point of using .env files like this is to keep it generic

@p2004a
Copy link
Copy Markdown
Collaborator

p2004a commented Feb 18, 2026

whole point of using .env files like this is to keep it generic

For me I always source them to have them as environment variables, so I always have export but that's just me, so I will fix that to make it more generic.

@burnhamrobertp
Copy link
Copy Markdown
Author

If its a net reduction in the code quality to change it, I'm not opposed to using it as you've used it, but perhaps we add a .env.example that lays out the proper format

@burnhamrobertp
Copy link
Copy Markdown
Author

Locally the web flow types are generating properly, although the dry run for syncing to web flow is missing the new property values, so I've missed something somewhere. Once I get the issue fixed, I'll have it pushed up

@burnhamrobertp
Copy link
Copy Markdown
Author

@p2004a this is ready for review when you have time

Copy link
Copy Markdown
Collaborator

@p2004a p2004a left a comment

Choose a reason for hiding this comment

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

As you upgrade to v3, we can also fix:

  • CollectionItemWithData is no longer needed, Webflow.CollectionItem defines fieldData again as non-optional.
  • fieldsToItem the TODO there was fixed, the as Webflow.CollectionItem can be removed.
  • Those id! are bad, but let's not fix it here, I've opened #590
  • Not for here, but also opened #591

const values = field.validations.options.map((o: any) => o.name);
case Webflow.FieldType.File:
case Webflow.FieldType.ExtFileRef:
// File references are returned as URLs
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think they are: https://developers.webflow.com/data/reference/field-types-item-values#file

Where did you get it from?

case 'Option': {
const values = field.validations.options.map((o: any) => o.name);
case Webflow.FieldType.File:
case Webflow.FieldType.ExtFileRef:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I cannot find any documentation for ExtFileRef as is the tradition with webflow api continuing to be undocumented mess. I would drop handling it then, and add handling only once we know what is the shape of data.

propsWrite = { type: field.isRequired ? 'string' : ['string', 'null'], ...desc };
break;
case Webflow.FieldType.Option: {
const validations = field.validations as any;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

eh, still bad... webflow/openapi-spec#3 (comment) (just comment, nothing to address here)

perspectiveShotUrl: string | null;
moreImagesUrl: string[];
mapHeightMin: number;
mapHeightMax: number;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

mapHeightMin must be handled also in:

  • isWebflowMapInfoEqual
  • WebflowMapInfo::constructor
  • WebflowMapInfo::generateFields

perspectiveShotUrl: (map.perspectiveShot.length > 0 ? `${imagorUrlBase}fit-in/2250x/filters:format(webp):quality(85)/${rowyBucket}/${encodeURI(map.perspectiveShot[0]!.ref)}` : null),
moreImagesUrl: map.inGameShots.map(i => `${imagorUrlBase}fit-in/2250x/filters:format(webp):quality(85)/${rowyBucket}/${encodeURI(i.ref)}`),
// Defaults from spring/cont/base/maphelper/maphelper/mapdefaults.lua
mapHeightMin: meta.minHeight,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You've put those properties in output of getDerivedInfo, let's read it from derivedInfo not meta.

cp .env.example .env
```

Populate the necessary values in .env
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe instead

For testing some scripts that push data to external services, setting env variables in .env might be also required

to make it clear it's not required for the most common cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants