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

Apply new origami focus styling to x-follow-button #694

Merged
merged 3 commits into from
Feb 3, 2023

Conversation

fenglish
Copy link
Contributor

@fenglish fenglish commented Jan 30, 2023

x-follow-button has the same issue as n-myft-ui's follow button after origami team released new focus styling across ft.com.

The detail is in this n-myft-ui PR.

This PR is to apply the same changes in n-myft-ui to x-follow-button.
We always need to update two repos (n-myft-ui & x-follow-button) when some changes in myftLozenge mixins.
x-follow-button was originally created to replace all the n-myft-ui follow buttons on ft.com however we haven't done that yet. It causes some features are unsynced because we forget to update two of them.

To avoid the issue, we should use n-myft-ui's myftLozenge mixins until we move all the follow button to x-follow-button.

This PR has two parts

  1. Remove own myftLozenge mixins, change to use n-myft-ui's mixins
    ⚠️ Need to check no difference between own ones and n-myft-ui ones

  2. Update n-myft-ui to the latest version to apply new focus styling from o-normalise & o-buttons


1. Remove own myftLozenge mixins, change to use n-myft-ui's mixins

No big difference when I replaced the own lozenge mixins to n-myft-ui ones.
Only this n-myft-ui change had been forgotten to sync to x-follow-button.

aria-pressed=false aria-pressed=true
Own lozenge mixins main main-aria-pressed
n-myft-ui lozenge mixins myworking-branch my-working-branch-aria-pressed

2. Update n-myft-ui to the latest version to apply new focus styling from o-normalise & o-buttons

aria-pressed=false aria-pressed=true
New focus styling Screenshot 2023-01-30 at 16 41 27 Screenshot 2023-01-30 at 16 41 42

We haven't moved all myFT follow buttons to x-follow-button on ft.com yet. This causes we need to update stylings on both n-myft-ui and x-follow-button when some changes require to myftLozengeTheme. Actually some of the feature in n-myft-ui is not synced to x-follow-button. To avoid this issue, applying myftLozenge mixin from n-myft-ui.
@next-team next-team temporarily deployed to x-dash-con-2059-apply-n-8qf4rz January 30, 2023 17:17 Inactive
@fenglish fenglish marked this pull request as ready for review January 31, 2023 09:19
@fenglish fenglish requested a review from a team as a code owner January 31, 2023 09:19
Copy link

@juancancela juancancela left a comment

Choose a reason for hiding this comment

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

lgtm

@fenglish fenglish merged commit 3d15a8e into main Feb 3, 2023
@fenglish fenglish deleted the CON-2059-apply-new-origami-focus-styling branch February 3, 2023 11:12
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