Skip to content

Re-work CSS parsing logic. - #18

Merged
jgerigmeyer merged 12 commits into
mainfrom
new-walker
Aug 31, 2022
Merged

Re-work CSS parsing logic.#18
jgerigmeyer merged 12 commits into
mainfrom
new-walker

Conversation

@jgerigmeyer

Copy link
Copy Markdown
Member

@jgerigmeyer

Copy link
Copy Markdown
Member Author

@jgerigmeyer jgerigmeyer left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@sanajaved7 @oluoluoxenfree I have a number of questions, and it may make sense to talk through them together on a call -- perhaps tomorrow after check-in?

Comment thread src/parse.ts Outdated
const { anchorName, anchorEdge, fallbackValue } = parseAnchorFn(node);
anchorFunctions.push({
floatingEl: rule.value,
floatingEdge: this.declaration.property,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We should check whether this is position-related -- if not, then we need a regular math polyfill for it (instead of positioning).

Comment thread src/parse.ts
Comment thread src/parse.ts Outdated
Comment thread src/parse.ts Outdated
Comment thread src/parse.ts Outdated
Comment thread src/parse.ts
@jgerigmeyer
jgerigmeyer marked this pull request as ready for review August 23, 2022 13:44
@jgerigmeyer

Copy link
Copy Markdown
Member Author

Putting this on hold for now, while we reconsider our approach to parsing stylesheets (see: https://oddbird.slack.com/archives/C03K136KX2A/p1661273459265879).

Comment thread src/parse.ts Outdated
Comment thread src/parse.ts
Comment thread src/transform.ts Outdated
Comment thread src/parse.ts Outdated
* main:
  chore(deps): Automated dependency upgrades (#20)
Comment thread src/fetch.ts
@mirisuzanne

Copy link
Copy Markdown
Member

@jgerigmeyer what's most helpful for me to understand or review here? Is a functional review helpful, or is there something I should be digging into code-wise?

@jgerigmeyer

Copy link
Copy Markdown
Member Author

@jgerigmeyer what's most helpful for me to understand or review here? Is a functional review helpful, or is there something I should be digging into code-wise?

@mirisuzanne There's not a lot functional to review yet (other than looking at what's logged to the console, and comparing that with the demo stylesheets). You could skim the code (mostly the changes in parse.ts and fetch.ts), and/or take a look at the console logged data object (which should represent an extraction of the relevant data from the stylesheets).

@jgerigmeyer

Copy link
Copy Markdown
Member Author

@sanajaved7 @mirisuzanne I think this is ready for review, then merge into #19?

@mirisuzanne mirisuzanne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looking good!

@jgerigmeyer
jgerigmeyer merged commit 60a4ab8 into main Aug 31, 2022
@jgerigmeyer
jgerigmeyer deleted the new-walker branch August 31, 2022 18:25
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.

4 participants