82 add rich text editor for comments etc #114
Conversation
…aly and now displays Markdown correctly
|
Editor below. There is text overlaying the editor that I am unsure what to do with. |
dougwaldron
force-pushed
the
82-add-rich-text-editor-for-comments-etc
branch
from
60d6fcb to
8dbbcbb
Compare
There was a problem hiding this comment.
@YoyoCaleb, this is a good start. There are several issues to work on, but I don't see any major obstacles yet. Let me know if you want to meet to go over any of these.
-
The output doesn't get formatted in the current PR version. I see you added the Markdig package, but it isn't used anywhere. Did you forget to push a commit?
-
Speaking of Markdig, you've added it to all projects in the solution. Only add NuGet packages where they're needed.
-
The SimpleMDE project has unfortunately been abandoned, but there are other options. EasyMDE is an updated and current fork of SimpleMDE. Another possibility is ink-mde.
-
The CSS you've added doesn't work well with light-mode. Ink-mde advertises that it is compatible with color schemes, though I haven't verified that. For EasyMDE, this custom CSS might work.
-
The overlapping label happens because I used Bootstrap floating labels. This can be removed.
-
Instead of adding the editor to all textareas, let's make it opt-in with an optional class.
-
An HTML page can only have one
<head>element, so adding one to the ".cshtml" pages is not allowed. The scripts can be added to the@section Scriptssection at the bottom of the page. Currently I don't have a similar section for the stylesheets but one can be added to the "_Layout" template. -
Subresource Integrity is important to site security when linking to third-party scripts. (This is why SonarCloud identified 5 Security hotspots.) Either host the files locally or add the SRI
integrityattribute. -
I had a bug that prevented the Edit page from working. I'll add a fix for that.
I'll put together a code suggestion to illustrate some of these items.
|
Hey @YoyoCaleb, while reviewing your PR, I'd suggest the following code changes: You can also review and apply these suggestions locally on your machine. Learn more about GitKraken Code Suggest
Join your team on GitKraken to speed up PR review. |
|
If possible, I would like to get into a call with you to discuss the changes. Thank you Doug! |
…thub.com/gaepdit/template-app into 82-add-rich-text-editor-for-comments-etc
…d> tags for the scripts and stylesheet. Non functional as of now
…king on styling with dark mode and options for EasyMDE
There was a problem hiding this comment.
Great progress, Caleb! This is turning out to be a lot more complex than I expected, but we're getting there. Here are some more tasks to work on:
- I'm sure you've heard the phrase, "never trust user input"! There's always a security risk when rendering user-generated content to the browser, and methods like
@Html.Raw()should raise red flags. Content should be sanitized before being displayed.- In the editor: The EasyMDE docs have an example for using a tool called "DOMPurify".
- On the back end: Sanitize rendered Markdown with HtmlSanitizer. This can be added to your
MarkdownToHtmlmethod.
- Dark mode is looking good, but the "preview" and "fullscreen" editor modes are still not compatible.
- Add
.UseBootstrap()to theMarkdownPipelineBuilder()to get built-in Bootstrap classes. - The "Notes" label is below the textarea. Unfortunately, Bootstrap's floating labels feature required the
<label>to come after the<textarea>element. Removing the "form-floating" class requires also moving the label above the textarea. (Ideally, though, the textarea template would use a floating label if MDE is not enabled.) - Markdig is only used in the WebApp project. So move the "Markdown.cs" file to "src/WebApp/Platform/Rendering" (new folder) and rename the file to "MarkdownHelper.cs" (it should always be the same name as the class).
- Use libman for all external JS libraries.
- HTML paragraphs (
<p>element) cannot contain other block-level elements. On the Details page, change the container for the Notes to<section class="border border-info p-4 rounded mb-3">. (I included extra formatting to set off the Notes section from the rest of the page.)
As usual, thanks and let me know if you have any questions!
…rkdown on the backend.
Moved Markdown.cs to WebApp from AppServices, Uninstalled HtmlSanitizer from AppServices and reinstalled in WebApp
…ted changes into libman file
… for Textarea's using MDE
…on with rendered HTML
|
Added rich text editor.
Edited Notes for "WorkEntry ID 10000000-0000-0000-0000-000000000000" to show that Markdown is expressed in HTML
You can also create your own WorkEntry to test the editor.