Fix initialization logic for web #219
Conversation
| child: FutureBuilder<bool>( | ||
| future: summernoteInit, | ||
| builder: (context, snapshot) { | ||
| if (snapshot.hasData) { | ||
| return HtmlElementView( | ||
| viewType: createdViewId, | ||
| ); | ||
| } else { | ||
| return Container( | ||
| height: widget | ||
| .htmlEditorOptions.autoAdjustHeight | ||
| ? actualHeight | ||
| : widget.otherOptions.height); | ||
| } | ||
| }))), |
There was a problem hiding this comment.
Now that you have added a FutureBuilder surrounding the entirety of the build method, do we need this secondary FutureBuilder, as well as the summernoteInit variable? It seems to me we can get rid of this.
There was a problem hiding this comment.
i haven't noticed this was used to indicate initialization, yes will have to remove it
There was a problem hiding this comment.
Sounds good, will re-review when you get a chance to remove it :)
There was a problem hiding this comment.
Hi @ahmednfwela , sorry for getting back to you so late - I just got around to testing this and found one issue.
| WidgetsBinding.instance!.addPostFrameCallback((timeStamp) { | ||
| setState.call(fn); | ||
| }); |
There was a problem hiding this comment.
I am having issues with this change when the caret selection position is changed on web. The toolbar will not update the values, e.g. font size will not update when the text selection position is changed. Is there any particular reason why you added the postFrameCallback? I don't recall there being any issues with this setState function.
There was a problem hiding this comment.
that's because calling setState during build throws an exception
but now that we moved initialization logic in a future builder i am not sure if this throws or not
but font size not updating is not related to the changes i made, i think
There was a problem hiding this comment.
I didn't see any exceptions when this was removed. Also removing this caused the font size to update correctly, the set state was not firing due to the post frame callback. Do you mind testing on your end and making sure there are no side effects to removing this?
this moves calling
initSummernotefrom theinitState(which doesn't support async initialization) to the build method using aFutureBuilderand anAsyncMemoizer