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

feat: text formatting #10

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: text formatting #10

wants to merge 5 commits into from

Conversation

mint-dewit
Copy link

@mint-dewit mint-dewit commented Feb 22, 2024

About the Contributor

This pull request is posted on behalf of the NRK.

Type of Contribution

This is a: feature

Current Behavior

New Behavior

Following text formatting should be available

  • Bold
  • Italic
  • Underline
  • Reversed (show black text on white background)
  • Hidden (does not output to prompter screen but shows in editor)
  • Coloured text (I've added red and yellow for now)
  • An indicator for changing graphics in the back screen

Testing Instructions

Ctrl-b, ctrl-i, ctrl-u, ctrl-r, ctrl-f10, ctrl-f and ctrl-k respectively should toggle the text. (Or cmd instead of ctrl on mac)

For colours the shortcut will cycle through the various options.

Other Information

There used to be a shortcut on ctrl-q, I think I removed that for testing because cmd-q quits the entire application. might have accidentally committed it. oops. I'm not sure how sold we are on the current shortcuts and not having a toolbar in place anyway.

Status

  • PR is ready to be reviewed.
  • The functionality has been tested by the author.
  • Relevant unit tests has been added / updated.
  • Relevant documentation (code comments, system documentation) has been added / updated.

@mint-dewit mint-dewit force-pushed the SOFIE-2799-text-formatting branch from 9b21a50 to 2865997 Compare February 23, 2024 12:54
@mint-dewit mint-dewit marked this pull request as ready for review February 23, 2024 12:57
@mint-dewit mint-dewit requested a review from a team February 23, 2024 12:57
if (state.peek(2) !== 'X)') return

state.consume()
state.consume()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a particular reason this is done twice?

Comment on lines 64 to 65
char += state.consume()
// char += state.consume()
Copy link
Contributor

Choose a reason for hiding this comment

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

Not consumed twice here, despite the comment?

@nytamin nytamin requested a review from jstarpl February 26, 2024 13:39
@@ -24,6 +27,13 @@ export class ParserStateImpl implements ParserState {
})
this.buffer = ''
}
setMarker = () => {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should be called pushLeaf (i.e. a node that has no children) and accept an argument of NodeBase? I was expecting setMarker to have something to do with ProseMirror markers.

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.

3 participants