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

Feature: Split test smaller lesson content font size. #3976

Merged
merged 1 commit into from
Jul 16, 2023

Conversation

KevinMulhern
Copy link
Member

Because:

  • We are forcing larger font size on all users when it should be left as a choice.
  • Larger text accentuates and makes other things harder to get right - it had a part to play in the difficulty styling our links.

When enabled:
Screenshot 2023-07-14 at 16 02 12

When disabled (the default):
Screenshot 2023-07-14 at 16 02 24

This commit:

  • Experiment with using a baseline 16px font size.
    • Only 50% of users will see it so we can monitor feedback.
    • The best feedback for this is probably no feedback. The right font size is table stakes, if no one notices the change it will likely be a good thing.

Because:
* We are forcing larger font size on all users when it should be left as a choice.
* Larger text accentuates and makes other things harder to get right - it had a part to play in the difficulty styling our links.

This commit:
* Experiment with using a baseline 16px font size.
  * Only 50% of users will see it so we can monitor feedback.
  * The best feedback for this is probably no feedback. The right font size is table stakes, if no one notices the change it will likely be a good thing.
@KevinMulhern KevinMulhern self-assigned this Jul 14, 2023
@KevinMulhern KevinMulhern added the Type: Enhancement Involves a new feature or enhancement request label Jul 14, 2023
@KevinMulhern KevinMulhern temporarily deployed to odin-review-app-pr-3976 July 14, 2023 15:09 Inactive
Copy link
Member

@ChargrilledChook ChargrilledChook left a comment

Choose a reason for hiding this comment

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

1 minor comment about the plan for follow up / future work, otherwise LGTM 🚀

Comment on lines +10 to +16
attr_reader :classes, :data_attributes, :current_user

def font_size
return '' if current_user && Feature.enabled?(:lesson_content_font_size, current_user)

'md:prose-lg'
end
Copy link
Member

Choose a reason for hiding this comment

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

Should we follow up and revert this if the experiment is successful?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, thats a great idea 💪

@KevinMulhern KevinMulhern merged commit 10c668a into main Jul 16, 2023
@KevinMulhern KevinMulhern deleted the feature/split-test-smaller-lesson-content-font branch July 16, 2023 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Involves a new feature or enhancement request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants