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

Render bold/italics/underline on SimpleTextDisplayer #2779

Merged

Conversation

muhammadharis
Copy link
Contributor

Also pertains to #2648.

[shakaCue]);
});

it('adds linebreaks when a linebreak cue is seen', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although it doesn't directly relate to the goal of this pull, I thought I might as well add a unit test for this as well.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you!

Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

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

Looks great! If you would add bold, it would be perfect.

@@ -82,8 +82,25 @@ shaka.text.SimpleTextDisplayer = class {
// Flatten nested cue payloads recursively. If a cue has nested cues,
// their contents should be combined and replace the payload of the parent.
const flattenPayload = (cue) => {
// TODO: If we want to support bold, italic, underline here, we would
// insert markup into the payload.
// Handle styles (currently underlines/italics). TODO support for color.
Copy link
Member

Choose a reason for hiding this comment

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

Could you add support for bold? Something like cue.fontWeight >= shaka.text.Cue.fontWeight.BOLD?

[shakaCue]);
});

it('adds linebreaks when a linebreak cue is seen', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Thank you!

small comment fix
@muhammadharis muhammadharis changed the title Render underline/italics on SimpleTextDisplayer Render bold/italics/underline on SimpleTextDisplayer Aug 11, 2020
Copy link
Member

@joeyparrish joeyparrish left a comment

Choose a reason for hiding this comment

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

Thank you!

@joeyparrish joeyparrish added the type: enhancement New feature or request label Aug 11, 2020
@joeyparrish joeyparrish added this to the v3.1 milestone Aug 11, 2020
@shaka-bot
Copy link
Collaborator

All tests passed!

@joeyparrish joeyparrish merged commit 91a284f into shaka-project:master Aug 11, 2020
@muhammadharis muhammadharis deleted the simpletextdisplayer-styling branch August 19, 2020 17:05
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jul 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants