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.

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

Overhaul AutoLayout Alignment property #95

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nkanigsberg
Copy link
Collaborator

@nkanigsberg nkanigsberg commented May 30, 2023

🚧 Work In Progress 🚧

TODO

  • Need to find solution for direction token issue. Since the direction property can be tokenized, the values aren't actually known until after styles are computed. And since we need to know the direction to correctly compute the alignment (for both the align-items and justify-content properties), this presents a problem. For the time being I've just added an escape where the alignment property isn't actually computed if passed anything but horizontal or vertical, which obviously is not a good long-term solution.
    • One possible solution is to create a constants object similar to library/foundations/generated/design-tokens.constants.ts, but containing values. From this we can compute the appropriate styles at each breakpoint... but this would require listening to the browser size.
    • Another possible solution is to attach classes to the autolayout depending on the direction which could be used to determine behaviour. More thought is needed on this one, CC @jasonsantos
  • We also need to update all instances (components and stories) where the alignment property has been used to make sure they are working correctly with the new schema

Screenshots

Screenshot 2023-05-30 at 3 40 59 PM Screenshot 2023-05-30 at 3 41 26 PM Screenshot 2023-05-30 at 3 41 54 PM Screenshot 2023-05-30 at 3 42 08 PM

@codesandbox
Copy link

codesandbox bot commented May 30, 2023

This branch is running in CodeSandbox. Use the links below to review this PR faster.


CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders | Preview

@github-actions
Copy link

Demo App Preview: https://d11mwnosflssyy.cloudfront.net/

@github-actions
Copy link

Code Coverage Report

Coverage after merging AutoLayout-alignment-overhaul into main

96.76%
Coverage Report
FileBranchesFuncsLinesUncovered Lines
../foundations/generated
   design-tokens.constants.ts100%100%100%
../foundations/generated/icons
   account-circle.tsx100%100%100%
   arrow-left.tsx100%0%75%4
   arrow-right.tsx100%100%100%
   chevron-right.tsx100%0%75%4
   close.tsx100%0%75%4
   event-note.tsx100%0%75%4
   figma.tsx100%100%100%
   github.tsx100%100%100%
   index.ts100%46.67%100%
   instagram.tsx100%100%100%
   linked-in.tsx100%100%100%
   menu.tsx100%100%100%
   schedule.tsx100%0%75%4
   settings.tsx100%0%75%4
   twitter.tsx100%0%75%4
   youtube.tsx100%0%75%4
components/auto-layout
   auto-layout.styles.ts72%100%100%..., 28, 62, 63, 64
   auto-layout.tsx100%100%100%
   auto-layout.types.ts100%100%100%
   index.ts100%100%100%
components/button
   button.styles.ts0%100%100%13
   button.tsx100%100%100%
   index.ts100%100%100%
components/footer
   footer.styles.ts100%100%100%
   footer.tsx100%100%100%
   index.ts100%100%100%
components/hero
   hero.styles.ts50%100%100%10
   hero.tsx100%100%100%
   index.ts100%100%100%
components/icon
   icon.styles.ts100%100%100%
   icon.tsx50%100%100%33
   index.ts100%100%100%
components/icon/utils
   index.ts100%100%100%
   with-icon.tsx100%0%40%10, 11, 23
components/image-text-item
   image-text-item.styles.ts100%100%100%
   image-text-item.tsx100%100%100%
   index.ts100%100%100%
components/image-text-list
   image-text-list.tsx100%100%100%
   index.ts100%100%100%
components/inline-link
   index.ts100%100%100%
   inline-link.styles.ts50%100%100%13, 19, 9
   inline-link.tsx66.67%100%100%44, 51
components/link-button
   index.ts100%100%100%
   link-button.styles.ts50%100%100%14, 19, 8, 9
   link-button.tsx83.33%100%100%43
components/link-icon
   index.ts100%100%100%
   link-icon.styles.ts50%100%100%14, 18, 24
   link-icon.tsx66.67%100%100%43, 52
components/nav
   index.ts100%100%100%
   nav.styles.ts50%100%100%33, 50
   nav.tsx66.67%80%95%85, 87, 89
components/nav-item
   index.ts100%100%100%
   nav-item.styles.ts100%100%100%
   nav-item.tsx100%100%100%
components/typography
   index.ts100%100%100%
   typography.styles.ts100%100%100%
   typography.tsx100%100%100%
utils
   design-tokens.utils.ts100%100%100%
   flatten.utils.ts100%100%100%
   index.ts100%100%100%
   styles.utils.ts100%100%100%

@nx-cloud
Copy link

nx-cloud bot commented May 30, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 05b2c34. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 3 targets

Sent with 💌 from NxCloud.

@nkanigsberg nkanigsberg mentioned this pull request Jun 6, 2023
1 task
@nkanigsberg nkanigsberg force-pushed the AutoLayout-alignment-overhaul branch from 3beeb3c to 05b2c34 Compare June 6, 2023 15:35
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.

1 participant