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

Add language assistance #12690

Closed

Conversation

assertchris
Copy link
Contributor

@assertchris assertchris commented May 8, 2024

Description

This PR adds language assistance helpers (like Hemingway) to the MarkdownEditor component.

Visual changes

demo

Functional changes

  • Code style has been fixed by running the composer cs command.
  • Changes have been tested to not break existing functionality.
  • Documentation is up-to-date.

@assertchris assertchris marked this pull request as draft May 8, 2024 09:19
@assertchris assertchris changed the title [WIP] Add language assistance Add language assistance May 8, 2024
@assertchris assertchris marked this pull request as ready for review May 8, 2024 09:30
@danharrin danharrin self-assigned this May 8, 2024
@danharrin danharrin added the enhancement New feature or request label May 8, 2024
@danharrin danharrin added this to the v3 milestone May 8, 2024
@assertchris
Copy link
Contributor Author

Just to note, I removed inline styles (replaced with tailwind classes) shortly before recording the demo but didn't build the stylesheet before recording. The dots are circular with a re-build of the stylesheet. In case you noticed and it bugs you as much as it does me.

const sentences = getSentenceDifficulty(line.text)
const lineNumber = cm.getLineNumber(line)
const lineElement =
this.$refs.editor.parentNode.parentNode.querySelectorAll(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shameful, I know. CM doesn't provide a way to get the line elements, and this event handler isn't given them either. Can't use renderLine (which does get a line element) because marking in that generates an infinite loop.

@danharrin
Copy link
Member

To be honest, I am a little overwhelmed with having to maintain something like this

@danharrin
Copy link
Member

I originally thought that your package would be doing a bit more of the heavy lifting and it was just a case of requiring it

@assertchris
Copy link
Contributor Author

assertchris commented May 8, 2024

Would it be better if that (roughly) 160 lines of component JS was moved to another lib? I suspect the template stuff is equally to blame for the apprehension; but I don't know how I could package that separately in a way that would keep it just as tightly coupled as it is now.

@danharrin
Copy link
Member

Yeah, is there a way you can bundle a CM plugin? Maybe you can inject the HTML through that plugin also? I'm not familiar there

@assertchris
Copy link
Contributor Author

I think there's still be significant stuff that needs to be elsewhere (templates, alpine state etc.)

@assertchris
Copy link
Contributor Author

Ok, I'll make another plan. Disappointing amount of work making it this far down a dead end road though. :/

@assertchris assertchris closed this May 8, 2024
@danharrin
Copy link
Member

Yeah, I really am sorry about that. Hopefully you can see it from our perspective too though. I'm happy to include it in core as long as we're not the ones responsible for maintaining that code.

@assertchris
Copy link
Contributor Author

Yep, no worries. Thanks for your time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants