Skip to content

Conversation

tylersticka
Copy link
Member

Overview

This PR installs a plugin for linting logical properties and applies those fixes across all existing stylesheets.

Please note that the intent of this PR is to enforce consistent property usage, not to optimize each and every component for different text directions and writing modes.

Screenshots

Screen Shot 2021-11-22 at 3 02 16 PM

Testing

  1. View a few patterns with "text flow" set to something other than the default to verify that properties have been updated.
  2. Observe basically every pattern in the deploy preview for regressions using the "default" text flow. I would most appreciate 👀 on Chrome, Edge and/or Safari (as Firefox is my primary browser and probably has disproportionate coverage).

@changeset-bot
Copy link

changeset-bot bot commented Nov 22, 2021

🦋 Changeset detected

Latest commit: f93f46a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cloudfour/patterns Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@netlify
Copy link

netlify bot commented Nov 22, 2021

✔️ Deploy Preview for cloudfour-patterns ready!

🔨 Explore the source changes: f93f46a

🔍 Inspect the deploy log: https://app.netlify.com/sites/cloudfour-patterns/deploys/619c221efd33b40007ae0ea6

😎 Browse the preview: https://deploy-preview-1595--cloudfour-patterns.netlify.app

@tylersticka tylersticka marked this pull request as ready for review November 22, 2021 23:12
@tylersticka tylersticka requested a review from a team November 22, 2021 23:12
@spaceninja
Copy link
Member

Code looks good!

Just finished testing for regressions in Edge. I'll test Safari next.

I should note that I'm not super familiar with these patterns, so while I don't see any obvious problems, it might be worth tapping Danielle or another designer to do a regression pass as well.

Copy link
Member

@spaceninja spaceninja left a comment

Choose a reason for hiding this comment

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

Safari looks good too.

I noticed two unrelated bugs in Safari (that are also visible in v-next):

  1. Large amount of white space below Overview blocks (visible on the Overview page and the Article Listing prototype). (#1596)

  2. Hovering an avatar on the Team List page turns the avatar square, in a weird flickering way that looks broken. (#1597)

I'll file bugs for those.

I'm going to approve this PR, but I'd still appreciate a designer who's more familiar with these patterns doing a quick regression sweep.

@tylersticka tylersticka requested a review from a team November 24, 2021 00:40
@dromo77
Copy link
Contributor

dromo77 commented Nov 24, 2021

I tested in Chrome since it looks like the other browsers have been covered. I didn't find any regressions, going to approve from a design standpoint.

@tylersticka tylersticka merged commit aa21527 into v-next Nov 24, 2021
@tylersticka tylersticka deleted the feature/lint-logical-props branch November 24, 2021 21:39
@github-actions github-actions bot mentioned this pull request Nov 24, 2021
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.

Make use of logical properties consistently
3 participants