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

Align #60

Open
wants to merge 14 commits into
base: matrix
Choose a base branch
from
Open

Align #60

wants to merge 14 commits into from

Conversation

alexf101
Copy link

@alexf101 alexf101 commented Jul 28, 2017

Support align* environments for displaying and editing multi-line equations.

To get this working with a minimum of code duplication, I've massaged the matrix environment slightly to have certain parameters configurable that were previously not. In particular, there's now an htmlColumnSeparator for adding non-editable visual content to separate the columns, which in this case is the ' = ' sign.

Customary GIF :)
multiline

Requires:

alexf101 added 9 commits July 12, 2017 17:09
…trix.

This lets the user cross over the top or bottom of the matrix naturally
without getting caught inside it, using the same config option as for
fractions.
By overriding moveTowards and moveOutOf on matrix and MatrixCell
respectively, we can avoid changing {up,down}Into into functions and so
keep these changes restricted to affecting the matrix environment.
@alexf101
Copy link
Author

Hi @eoghanmcilwaine!

I've merged master into this and added a unit test and visual test.

This is the visual test output:

screen shot 2017-10-25 at 4 31 17 pm

However, it's not quite right yet - I'd really like it to respond to click events by placing the cursor in the near row. At the moment, it appears to put the cursor in the top row if you miss. I was wondering if you have any idea how to accomplish that in MathQuill?

Copy link

@eoghanmcilwaine eoghanmcilwaine left a comment

Choose a reason for hiding this comment

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

Hi @alexf101, nice work here.

To answer your question - the cursor positioning after a click is set by the seek controller method. It basically puts the cursor inside/beside the math object you've clicked.

Because the =s down the middle are just some extra html inserted into the {align*} table rather than being math objects in their own right, all seek knows is that you've clicked somewhere on the {align*} object. So the cursor is inserted at the top of the table somewhere.

I think the solution would probably be to allow mathquill to parse and insert the =s as normal (well not exactly as normal because it'd be great to keep the extra <td> around the = somehow).

Incidentally this could help reduce the divergence between Matrix and Align* as the column delimiter would no longer be different. I'm just doing a review of your code - it's a good solution in general but one thing I'm not sure about is adding Align-specific conditional stuff to Matrix (it's like this classic code smell). There's probably a neater way. I'm having a go at using a common abstract ancestor for Align and Matrix but there could be a slicker way.

@@ -863,7 +863,7 @@ LatexCmds.begin = P(MathCommand, function(_, super_) {
var string = Parser.string;
var regex = Parser.regex;
return string('{')
.then(regex(/^[a-z]+/i))
.then(regex(/^[a-z*]+/i)) // LaTex uses * as a convention for alternative forms of a command, usually 'without automatic numbering'

Choose a reason for hiding this comment

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

Hey @alexf101, am I right in saying the asterisk can only come at the end? So it'd be something like ^[a-z]+\*?

Copy link
Author

Choose a reason for hiding this comment

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

Seems reasonable - I'm no latex expert, but I've only ever seen it used at the end.

Copy link

@zyLear zyLear Aug 17, 2019

Choose a reason for hiding this comment

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

@eoghanmcilwaine can you give me the mathquill.js and mathquill.css . I can't build

error message:
' undefinedError: Cannot read property 'eval' of null '

Copy link

Choose a reason for hiding this comment

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

@eoghanmcilwaine thank you very much

Copy link

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

image

// differently in latex. Nevertheless, we want it to render as a table so it's convenient to extend Matrix.
Environments['align*'] = P(Matrix, function (_, super_) {
_.environment = 'align*';
_.extraTableClasses = 'rcl';

Choose a reason for hiding this comment

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

@alexf101 I think all mathquill classes start with mq- so best to stick with that convention here.

Copy link
Author

Choose a reason for hiding this comment

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

Ok

test/visual.html Outdated
\begin{align*}
y &= mx+c \\
y-c &= mx \\
\frac{y-c}{m} &= x

Choose a reason for hiding this comment

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

Can we encode the ampersands as &amp;? Just so it's really unambiguous.

Copy link
Author

Choose a reason for hiding this comment

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

Sure

- Tighten regex to only allow * at the end
- prefix mathquill class names with 'mq-'
- Use '&amp;' in HTML files instead of & for literal ampersand.
@alexf101
Copy link
Author

Ok, thanks so much for that information!

So the mental model I have is that the nearest parent object of any cell in the align* environment is the row (which probably differs somewhat from a Matrix in general, where some clients might consider the column as the parent, or the matrix itself).

Would it make sense (and solve the problem of click events on = moving the cursor to the top row instead of the nearest row) to define a MatrixRow extending MathBlock and reimplement using that?

It seems like a fairly deep reimplementation would be required to introduce a MatrixRow to the object - I would view this as a practical reason to follow your advice and separate the align environment from the matrix implementation, even if most of the code ends up being similar.

I'll try and have a crack at it - let me know if this approach makes sense to you, or something else :)

@eoghanmcilwaine
Copy link

eoghanmcilwaine commented Jan 2, 2018

Cheers @alexf101. I think you're spot on about rows making more sense as separate objects. It's a pretty big change though.

I came up with a solution to the cursor-in-top-row issue without changing seek or matrix - commit here. It's kind of hacky but self-contained at least.

By the way, I separated the matrix and align logic in another commit.
The diff is a bit long but the change is fairly simple: I've created a common ancestor "TabularEnv" that both matrix and align* inherit from. So for example TabularEnv has an addRow method but only Matrix has addColumn. See what you think, I reckon it's better this way as it removes some conditional complexity.

@alexf101
Copy link
Author

@eoghanmcilwaine Looks awesome! Let's move to the align-alternative branch :)

Is there a PR for merging align-alternative into MathQuill that I can jump on?

So sorry about the long delay - the usual story of too aggressive email filters on GitHub notifications, working on other projects, etc. I really appreciate the interest you've taken in this!

@alexf101
Copy link
Author

@eoghanmcilwaine I tried it out, and it seems to... kind of work? I have to click pretty precisely on the middle of the equals sign for it to stay in the same row. Is that your experience too, or did I make some mistakes merging your branch over our fork?

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

Successfully merging this pull request may close these issues.

3 participants