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

0.2.0 API Redesign #10

Closed
znjameswu opened this issue Sep 18, 2020 · 17 comments
Closed

0.2.0 API Redesign #10

znjameswu opened this issue Sep 18, 2020 · 17 comments

Comments

@znjameswu
Copy link
Owner

znjameswu commented Sep 18, 2020

Current 0.1.x's API design is more of a makeshift design that has a lot of oversight and redundancy which is later proved to be not so useful. As the development for follow-up feature goes deeper and I'm having a better understanding for the API demand, it is probably the time to stablize the API and make some unavoidable breakings. The proposed API change takes inspirations from other projects from zefyr and CaTeX.

Widget variants

The first part is to break up widgets into MathView, MathSelectable, and (future) MathEditable, instead of one FlutterMath. This will avoid extra performance cost and API surface when only MathView is needed.

FlutterMath will be deprecated and then removed.

TextStyle over Options

TextStyle will be adopted as the parameter in exposed APIs to replace Options. This will have several implications:

  1. TextStyle.fontSize will be the logical pixel count of 1 cssEm (which is the fontSize passed to Text widget under MathStyle.display and normal size). This will affect all relative length units such as mu, etc. A constant defaultMathFontSize will be provided.
  2. Another parameter lpPerInch will be used to decide the length of absolute units. If lpPerInch is null, then absolute units will follow relative units and resize on different TextStyle.fontSize (Just like current baseSizeMultiplier's behavior). A function double defaultLpPerInch(double fontSize) will be provided to calculate the effective lpPerInch when set to null.
  3. As such, baseSizeMultiplier will be removed.

The final API will look like

MathView.fromTex(
  r'\frac a b',
  style: TextStyle(fontSize: 42),
  // parserSettings: TexSettings(),
  // lpPerInch: defaultLpPerInch(42),
  onErrorFallback: (err) => Text(err.toString()),
)

Renaming classes

Some old classnames are prone to naming collision in user code. The following will be an incomplete list of renaming:
Options -> MathOptions
Settings -> TexSettings
SizeMode -> MathSize

Grouping exports

Now exports will be grouped into three libraries
package:flutter_math/widgets.dart exports basic widgets
package:flutter_math/ast.dart exports AST nodes and necessary tooling
package:flutter_math/tex.dart exports TeX parser and useful settings

Updating Plan

Aside from the variant widget proposal, all other changes are breaking.

Currently MathSelectable and a bunch of optimizations is in the works. Widget variants, MathSelectable and those optimizations will land in 0.1.9. 0.1.9 will also mark FlutterMath as deprecated. However, none of these will be breaking.

After 0.1.9 is rolled, 0.2.0 will come out very shortly, with all the above breaking changes. 0.2.0 will also remove FlutterMath.

@znjameswu
Copy link
Owner Author

@creativecreatorormaybenot Please let me know your thoughts on this design. They are heavily inspired by your CaTeX project and you are of course more experienced on this subject.

@creativecreatorormaybenot
Copy link
Contributor

@znjameswu Thanks for pinging me - I will take a look probably on Tuesday; until then I am busy, but I will not forgot to take a look.

It looks very exciting 🔥

@znjameswu
Copy link
Owner Author

logicalPpi seems like a better name than lpPerInch.

@creativecreatorormaybenot
Copy link
Contributor

@znjameswu Yes, certainly. I actually have no objections to verbose variable names (e.g. logicalPixelsPerInch), but I do see the advantage of shortening these when they are used a lot + only referenced internally.

I know I said Tuesday, but I think by this week is a better estimate - so I will get back by Sunday for sure; preferably earlier :)

@znjameswu
Copy link
Owner Author

@znjameswu Yes, certainly. I actually have no objections to verbose variable names (e.g. logicalPixelsPerInch), but I do see the advantage of shortening these when they are used a lot + only referenced internally.

I know I said Tuesday, but I think by this week is a better estimate - so I will get back by Sunday for sure; preferably earlier :)

Thanks for your comment. No need to hurry! Right now I only finished a MathSelectable prototype. But to provide sensible contents to the clipboard, it still needs a TeX encoder. So, still some time to go.

The internal implementation on the master branch has already been changed to using TextStyle.fontSize and logicalPpi. The code is a lot simpler and performant now. baseSizeMultiplier was really a makeshift design and complicate stuff. Thank you for this API idea.

@creativecreatorormaybenot
Copy link
Contributor

Here are some of my thoughts:

The first part is to break up widgets into MathView, MathSelectable, and (future) MathEditable, instead of one FlutterMath. This will avoid extra performance cost and API surface when only MathView is needed.

I like the idea for the development side of the API. In terms of usage, I feel like it would be nice to have one widget (at least for view-only and selectable). For example, Flutter has Text and SelectableText, however, that feels somewhat counterintuitive in some scenarios.
So, I do like the separation and I would just add another widget to the table: something that takes in selectable and editable parameters and then just uses one of the three other widgets - I would still expose the three new widgets 👍

Furthermore, I am not sure if MathView is the best name. It reminds me of web view, but I am not sure about a better name in that style. What I would for sure say is flip the Selectable and Editable parts - I think in English it makes more sense to name the adjective first (see EditableText).

Now exports will be grouped into three libraries
package:flutter_math/widgets.dart exports basic widgets
package:flutter_math/ast.dart exports AST nodes and necessary tooling
package:flutter_math/tex.dart exports TeX parser and useful settings

I think that a single import should be provided for normal API use because it is cumbersome having to import two libraries for using one widget. More precisely, when I want to use a MathView (according to the proposed naming), I expect that if I import the library for that once, I also have imported all the settings. It might be that your widgets.dart export already intended to do that.

Moreover, I think that a library export is idiomatic. So maybe you want to have a library export that just redirects to widgets.dart. Or maybe the library export exports all of the files.
It just feels like the library import is something useful.

Conclusion

I really like these changes 🔥 and I am already really excited to see them in action 🚀

Note

I already took a look at some of the commits you pushed - maybe a little bit of advice here: it does make sense to not push every commit to master (makes it easier to view changes and keep track of what actually happens between versions). For example, you could have a branch for each new update and then create a pull request to merge that as a new version. This will allow you to see the bundled changes and then squash merge that onto master, which will make for a really clean commit history on master (and also enable visitors to see the current code of the published version).

@znjameswu
Copy link
Owner Author

Here are some of my thoughts:

The first part is to break up widgets into MathView, MathSelectable, and (future) MathEditable, instead of one FlutterMath. This will avoid extra performance cost and API surface when only MathView is needed.

I like the idea for the development side of the API. In terms of usage, I feel like it would be nice to have one widget (at least for view-only and selectable). For example, Flutter has Text and SelectableText, however, that feels somewhat counterintuitive in some scenarios.
So, I do like the separation and I would just add another widget to the table: something that takes in selectable and editable parameters and then just uses one of the three other widgets - I would still expose the three new widgets 👍

Furthermore, I am not sure if MathView is the best name. It reminds me of web view, but I am not sure about a better name in that style. What I would for sure say is flip the Selectable and Editable parts - I think in English it makes more sense to name the adjective first (see EditableText).

Now exports will be grouped into three libraries
package:flutter_math/widgets.dart exports basic widgets
package:flutter_math/ast.dart exports AST nodes and necessary tooling
package:flutter_math/tex.dart exports TeX parser and useful settings

I think that a single import should be provided for normal API use because it is cumbersome having to import two libraries for using one widget. More precisely, when I want to use a MathView (according to the proposed naming), I expect that if I import the library for that once, I also have imported all the settings. It might be that your widgets.dart export already intended to do that.

Moreover, I think that a library export is idiomatic. So maybe you want to have a library export that just redirects to widgets.dart. Or maybe the library export exports all of the files.
It just feels like the library import is something useful.

Conclusion

I really like these changes 🔥 and I am already really excited to see them in action 🚀

Note

I already took a look at some of the commits you pushed - maybe a little bit of advice here: it does make sense to not push every commit to master (makes it easier to view changes and keep track of what actually happens between versions). For example, you could have a branch for each new update and then create a pull request to merge that as a new version. This will allow you to see the bundled changes and then squash merge that onto master, which will make for a really clean commit history on master (and also enable visitors to see the current code of the published version).

Thanks for your comments. They are very inspiring.

Widget separation

Yes, I also thought about flipping the name. But I can't find a proper name for a static, non-selectable widget. A non-selectable widget is kind of important, because its implememtation was designed to avoid any state management cost, while selectable ones shares the same performance penalty as the editable.

I think MathView is actually a decent name. They use TexView in flutter_tex and ZefyrView in zefyr.

I think editable widget can take the parameter to controll modes and probably change modes internally.

Library exports

I will follow your advice. A main export containing all frequently used utilities and other exports for less frequently used ones.

Commit style

I must apologize for the makeshift style of commits on master. As the majority of the work has finished, I'll take a better style of commit management.😅

@znjameswu
Copy link
Owner Author

Progress Update

Selectable Widget

With commit 95410f6, selectibility-related feature is about to complete. The only blocking issue is this DDC bug flutter/flutter#66122 causing severe, undebuggable runtime error during web debugging on (currently) all Flutter channels. This is due to the code patterns used in implementing selectable widgets (mixin pattern to be specific, which I am reluctant to refactor for a compiler bug). I think it's best to hold off until this bug is solved by Flutter's cherrypick.

Features already finished:

  • Selection overlay
  • Selection handles
  • Selection toolbars
  • 'Right click to copy' on Web (this hack is reeeeeeally agonizing)

Features yet to be completed

  • Spec-compliant behavior on different platforms
  • A bunch of style parameters (cursorWidth, etc.).
  • Keyboard-driven selection (shortcuts and caret movement, etc)
  • Tests

Encoder

The encoder now support the encoding of some most frequently used node types. The output will be imprecise in a lot of cases, but it is optimized and concise.

New API

After some coding, I find your suggestion of naming to be better after all @creativecreatorormaybenot 😄. SelectableMath is indeed better. But what name should we give for the static, non-selectable variant? StaticMath, BlockMath, MathSpan or some better name? Please let me know your thought.

@creativecreatorormaybenot
Copy link
Contributor

@znjameswu The selection feature is really impressive 👌 🚀 And I know the pain - I have also struggled with DDC issues a lot in the past year; really looking forward to web becoming stable.

What is the hack for right click to copy? Are you talking about the fact that Flutter does not support regular context menus on web?

Regarding the naming, I think that given how some framework widgets are named, Math might be a decent widget name. Although, it could potentially clash with some math utility (although this would be a weird class name - I mean what class apart from a UI widget covers all math).
StaticMath does not logically exclude the selection feature. Maybe MathBody would be an option (similar to flutter_markdown).
I genuinely think going for just Math is the way to go (if there is no common class with this name) because it allows users to think like this: "OK I have the Math widget, where I want to put in some math to display it."

@creativecreatorormaybenot
Copy link
Contributor

I see: you used a textarea hack - can you walk me through that? It seems really interesting to me since I have also done a bunch of HTML element stuff in Flutter web 👍

@znjameswu
Copy link
Owner Author

I see: you used a textarea hack - can you walk me through that? It seems really interesting to me since I have also done a bunch of HTML element stuff in Flutter web 👍

Sure, textarea is to mimic how Flutter Web hacks to make its SelectableText have a 'copy' option. When you create any EditableText (no matter from SelectableText or TextField) and focus on it, a corresponding textarea will be generated. That textarea is 100% transparent, but it is binded (via TextInputConnection) to the editing value. If positioned correctly (by updating position at every frame) and having the correct font and size, this textarea will be 100% identical to the text on canvas (size, layout, value, selection range). When you right clicking at the text, you are actually dealing with the textarea. This is how Flutter hacks to have a correct context menu.

For equations, the textarea can never be 100% identical to Flutter's layout. Fortunately, Flutter Web clips every element including textarea to corresponding widget size. So if I make the textarea horrendously big, aligned at center, and always at an "all-selected" state, then after the clip, the widget itself will still behave like a selected textarea.

@creativecreatorormaybenot
Copy link
Contributor

@znjameswu That is very interesting thanks 👌

@znjameswu
Copy link
Owner Author

0.2.0 has been released. Though dartdoc is currently broken at pub.dev dart-lang/pub-dev#4166

@creativecreatorormaybenot
Copy link
Contributor

@znjameswu Awesome news 🚀

@creativecreatorormaybenot
Copy link
Contributor

@znjameswu I forgot to send the comment: I tried the new release and I think it works awesome.

I really appreciate the new project structure, Git workflow, and naming 👌

It seems like there are still some problems with SelectableMath, i.e. selection in general, however, I honestly think that this is not a big deal - just having part of the feature is already amazing and you went a long way with this 🚀

@znjameswu
Copy link
Owner Author

@creativecreatorormaybenot
Thank you! I appreciate that you like my work!

I agree that the selection part is very rough and unpolished. I initially thought it would be easy to just copy from Flutter's built-in widgets, but later found Flutter's implementation to be rough as well. My current plan is to halt feature development, as selection and IME handling problems are still hard to tackle at now, and wait for Flutter team to come up with nicer solutions. But I will still do the NNBD migration and bugfixes. Again thank you for your interest in this work!

@olof-dev
Copy link

How about MathText for the widget? Like RichText, but math-enabled.

creativecreatorormaybenot added a commit to simpleclub-extended/flutter_math that referenced this issue Sep 20, 2021
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

No branches or pull requests

3 participants