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

Playhead text customization #508

Merged
merged 5 commits into from
Oct 11, 2023
Merged

Conversation

amber-huo
Copy link
Contributor

#507

This adds two options for customizing playhead text:

  • background color
  • padding around the text

@chrisn
Copy link
Member

chrisn commented Oct 6, 2023

Thank you! With the padding option, could this apply to both the left and right of the text, at the moment it looks to be only on the right. What do you think?

Another thing I would suggest is rename the playheadBkgrdColor as playheadBackgroudColor, so it's not abbreviated.

@amber-huo
Copy link
Contributor Author

Thank you! With the padding option, could this apply to both the left and right of the text, at the moment it looks to be only on the right. What do you think?

Another thing I would suggest is rename the playheadBkgrdColor as playheadBackgroudColor, so it's not abbreviated.

Sure, took both your suggestions.

@chrisn
Copy link
Member

chrisn commented Oct 6, 2023

Thank you! I'm just doing some testing. There are a couple of other minor things I'd like to consider. One is, when the playhead is close to the right-hand edge of the view, the playhead text moves to the left of the playhead, so ideally this would also take into account the padding. The other is that there's a really small gap between the playhead and the padding, would it look better if we could remove this?

@amber-huo
Copy link
Contributor Author

Ok, I adjusted for padding when the text moves to the left, and removed the gap. The padding has a default of 2 so the text should look the same as before when there is no custom background or padding set.

@chrisn
Copy link
Member

chrisn commented Oct 11, 2023

This looks great, thank you!

@chrisn
Copy link
Member

chrisn commented Oct 11, 2023

The only final things are to add some documentation and a few really minor nitpicks on the code. Would you prefer if I do those, or should I suggest changes?

@amber-huo
Copy link
Contributor Author

I'm cool if you go ahead and do those! Thanks.

@chrisn chrisn merged commit 28c2d6f into bbc:master Oct 11, 2023
1 check passed
@amber-huo amber-huo deleted the playhead-bkgrd-color branch October 20, 2023 03:04
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.

2 participants