Markdown: Full control over EasyMDE

[WolfgangKluge]

Is your feature request related to a problem? Please describe.
I try to configure the MDE instance but I don't have full control over the options you provide to the EasyMDE constructor.

Especially I miss spellChecker (bit like #1316), but I'm interested in status and renderingConfig, too.

Describe the solution you'd like
Every (or at least most) options EasyMDE provides, should be configurable by the Markdown editor.
https://github.com/Ionaru/easy-markdown-editor#options-list

[stsrki]

We tried to include as many features from MDE that we found interesting at the time, or that we needed for our own projects. Others will come in time, but v1.0 is already neer release and we cannot have much time for new features. Our focus is now is the stability and API cleaning.

In case you have time to help with this it would be a lot faster.

[WolfgangKluge]

I'd be glad to. The only thing I'm struggling with is the very tight coupling between EasyMDE and Blazorise.
Since everything else in the markdown editor component is tightly coupled to EasyMDE, too, this might be fine (just leaves a bad feeling).

[WolfgangKluge]

One question before I can start.
EasyMDE has default values for most of the options.

You repeat these default values in your implementation, e.g.

Blazorise/Source/Extensions/Blazorise.Markdown/Markdown.razor.cs

Lines 343 to 352 in a14106b

	/// <summary>
	/// If set to false, disable line wrapping. Defaults to true.
	/// </summary>
	[Parameter] public bool LineWrapping { get; set; } = true;
	/// <summary>
	/// Sets the minimum height for the composition area, before it starts auto-growing.
	/// Should be a string containing a valid CSS value like "500px". Defaults to "300px".
	/// </summary>
	[Parameter] public string MinHeight { get; set; } = "300px";

But you don't do that for all of the options, e.g.

Blazorise/Source/Extensions/Blazorise.Markdown/MarkdownErrorMessages.cs

Lines 9 to 12 in a14106b

	/// <summary>
	/// The server did not receive any file from the user. Defaults to You must select a file.
	/// </summary>
	public string NoFileGiven { get; set; }

(in fact, the whole ErrorMessages property is null by default)

I think the repetition of the default value is superflous in every case. The only "benefit" is, that updates on the default value of EasyMDE does not bother the blazorise user. Sometimes this is good, sometimes you prevent the user from new features/bug fixes, sometimes you run into a new bug (value type of property has changed - but that's always an error no matter if optional or not).

Personally, I would define every property as optional (even booleans with a default value of false). But it's your project. Therefore: should I repeat the default values of all the top-level properties (without the complex properties) like the currently available options in the current source? Or is it ok to define all the values (including the existing) as optional?

[stsrki]

You should define them with the default value, same as in EasyMDE. If they are left to the C# defaults then EasyMDE might not be initialized properly once the options are assigned on the JS side.

[WolfgangKluge]

My C#-default would always be null - but I've seen the problem e.g. here:
https://github.com/Ionaru/easy-markdown-editor/blob/50d36800174e8f0ac8a3406ea0fdfa3ccedd7fa2/src/js/easymde.js#L1751

This could be solved by unsetting (delete) null-properties in the javascript options object - but there are some properties (e.g. shortcuts), where null overrides the default behavior.. Oh snap.

Ok, I'll take the same approach as in the current code :)

[WolfgangKluge]

Final question regarding to the same null-problem:

I figured out an approach to use null values everywhere and still don't have problems with the EasyMDE initialization (since the values are just not assigned).

This will also solve a problem (at least I think it is one) with https://github.com/Megabit/Blazorise/blob/master/Source/Extensions/Blazorise.Markdown/MarkdownErrorMessages.cs, where one cannot use this option like

<Markdown
    ErrorMessages="@(new() {
        FileTooLarge = "Too large!"
    })"/>

(By using this, at the moment all the error messages but the given one are empty due to there are no default values). This would be true for every other complex option.
This is a problem at the moment where Blazorise extend such a options object and the end user is not really aware of this change.

The way would be:

• create a Options-Class (instead of using a anonymous type) for the serialization
• add [JsonIgnore( Condition = JsonIgnoreCondition.WhenWritingNull )] to every property where null is not used to override the default bevahior (not really nice, but I think it's ok)
• in the JavaScript part use the options directly (without copying all the values (that would be nice)
• append properties that are not serialized directly (e.g. errorCallback)
• optional: use [JsonPropertyName( "..." )] instead of renaming while copying and [JsonConverter(..)] for properties where multiple types are allowed (e.g. false and string[]).

At the end, with this solution there would be less repetitions in the code and you don't have to keep track of minor changes in the default values in EasyMDE.

Alternatively, Blazorise has to provide default values for every configuration option - not only the root level ones.

I know you've already answered, but I just have to ask again. (Sorry for that)

[stsrki]

Yes, by having an options class we would have more control over what and how they get serialized. I chose anonymous because it was easier to do at the time. Also, by having default values sent from the Blazorise side we have are more explicit by intention. Otherwise, we depend on what is the default by the EasyMDE which isn't always clear.

All in all don't lose too much time on adding an option class. At least for now. We can always revisit it for next versions.