-
Notifications
You must be signed in to change notification settings - Fork 14
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
Header myaccount updates #1646
Header myaccount updates #1646
Conversation
d900bcb
to
757dc11
Compare
@@ -20,7 +20,7 @@ | |||
} | |||
|
|||
/// More visually decorated link styles | |||
@mixin _oHeaderFancyLink() { | |||
@mixin _oHeaderFancyLink($iconVariant: false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewer notes: I am not sure on this approach but I wanted to suggest it to start the discussion.
The new Sign In and My Account link has very similar styles to that of the oHeaderFancyLink
links which are historically just used on the row below and not used in this top row.
The Sign In / My Account link also introduces an icon to the link but having the fancy link hover state underline go under the icon looked wrong. Therefore the design stated for that hover state to stop before the icon.
So I have achieved that by having a new argument in this mix that allows me to alter the hover state.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: Lets be less DRY with the my account link styles
height: $o-header-sticky-height; | ||
min-height: $o-header-sticky-height; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewer notes: Here I found that the content of the top row was constrained by this height. It seemed more flexible to have it as a min height so the contents can also increase it if needed. I am not sure if theres any undesirable side effects to this?
@@ -131,20 +131,20 @@ | |||
} | |||
} | |||
|
|||
.o-header__top-icon-link--myft { | |||
.o-header__top-icon-link--user { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewer notes: These styles are used by the Sign In / My Account link in the top right. For now I have taken the approach of reusing mixins that are already in use elsewhere in the header. I am not sure if this is the correct approach but it will aid conversation on it
components/o-header/src/tsx/top.tsx
Outdated
<MyAccountOrSignInLink | ||
item={myAccountAction} | ||
variant={variant} | ||
className="o-header__top-link--hide-m" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: This needs tidying up
components/o-header/src/tsx/top.tsx
Outdated
return ( | ||
<a | ||
className={`o-header__top-link ${className}`} | ||
className={`o-header__top-icon-link o-header__top-icon-link--user`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: Lets be less DRY with the my account link styles
fc7a29f
to
fd11a43
Compare
After a successful AB test to change the My Account & myFT entry point links we are rolling this out as the default behaviour. Details of the previous AB test can be found in the page-kit PR Financial-Times/dotcom-page-kit#1017 The changes included in this roll out are 1. Swapping the position of myFT and Settings & Account entry points, so Setting & Account is at the top and MyFT is below 2. Changing the name of "Settings & Account" to "My Account" 3. Adding a "user" icon next to the My Account and Sign in links 4. Adding "Feed" to the myFT link As well as the successful test these changes are based on user research that found 1. Users think the myFT logo is just branding and not a link 2. Settings and Account was not obvious place to go as the main hub for your Account 3. My Account was a more common term for that hub 4. The user icon help to distinguish this as your space
f74af56
to
dda6788
Compare
Description
Roll out the variant of the
experimentalAccountEntryTest
AB test as the default permanent behaviourJustification
In early 2024 the Retention team ran an AB test to try and improve the user experience of the header by trying to reduce confusion of the difference between Settings & Account and myFT. This test was successful so we are now rolling these changes out to all users. The changes that are relevant to this PR are
Settings & Account
toMy Account
myFT
tomyFT Feed
Screenshots
Anonymous user - Previous layout
Anonymous user - New layout
Signed in user - Previous layout
Signed in user - New layout
Reviewer considerations
a. Original AB Test PR
b. Origami navigation service data changes
Ticket
https://financialtimes.atlassian.net/browse/ACC-3083