10000 fix: Correct for negative margin in integrand lower limits. by ronkok · Pull Request #2987 · KaTeX/KaTeX · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: Correct for negative margin in integrand lower limits. #2987

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
May 12, 2021
Merged

fix: Correct for negative margin in integrand lower limits. #2987

merged 13 commits into from
May 12, 2021

Conversation

ronkok
Copy link
Collaborator
@ronkok ronkok commented May 5, 2021

Fixes #2986.

@codecov-commenter
Copy link
codecov-commenter commented May 5, 2021 •
edited
Loading

Codecov Report

Merging #2987 (9ec90fc) into master (1ed90e4) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2987   +/-   ##
=======================================
  Coverage   93.95%   93.95%           
=======================================
  Files          87       87           
  Lines        6333     6339    +6     
  Branches     1311     1313    +2     
=======================================
+ Hits         5950     5956    +6     
  Misses        353      353           
  Partials       30       30
Impacted Files Coverage Δ
src/functions/utils/assembleSupSub.js 96.15% <100.00%> (+1.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ed90e4...9ec90fc. Read the comment docs.

@ronkok
Copy link
Collaborator Author
ronkok commented May 5, 2021

Screenshotter is acting up again. I have updated images (with output from Screenshotter) and now Screenshotter has decided not to approve them. I am unable to proceed.

@edemaine
Copy link
Member
edemaine commented May 6, 2021

@ronkok Tests are also failing. Here's an error message:

● A macro expander › should build \overset and \underset

    Expected the expression to success building:
      "\\overset{f}{\\rightarrow} Y"
    Instead, it threw:
      TypeError: Cannot read property 'type' of null
          52 |  */
          53 | const getBaseElem = function(group: AnyParseNode): AnyParseNode {
        > 54 |     if (group.type === "ordgroup") {
             |               ^
          55 |         if (group.body.length === 1) {
          56 |             return getBaseElem(group.body[0]);
          57 |         } else {

@ronkok
Copy link
Collaborator Author
ronkok commented May 6, 2021 •
edited
Loading

Okay. All tests pass. Should be ready to go.

The call to utils.isCharacterBox() protects against placing this new spacer when it is not needed. If the bottom limit is sufficiently narrow, then the integrand is wider and it will control the horizontal alignment. Of course, we don't actually know the width of the bottom limit, but a test for a single character is a pretty good proxy.

edemaine
edemaine reviewed May 9, 2021
View reviewed changes
Copy link
Member
@edemaine edemaine left a comment

Choose a reason for hiding this comment

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

One question: Do we need to handle an offset for the sup too? Or is this just a sub problem?

@ronkok
Copy link
Collaborator Author
ronkok commented May 9, 2021

I don't think that any adjustment to sup is needed. Only sub gets a negative margin applied to it.

edemaine
edemaine reviewed May 11, 2021
View reviewed changes
Copy link
Member
@edemaine edemaine left a comment

Choose a reason for hiding this comment

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

This looks great. I have one idea for improvement, which you are free to disagree with. Then I think we can merge.

@ronkok
Copy link
Collaborator Author
ronkok commented May 11, 2021

Okay. Gotta clean test.

edemaine
edemaine approved these changes May 12, 2021
View reviewed changes
Copy link
Member
@edemaine edemaine left a comment

Choose a reason for hiding this comment

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

Great, thanks!

@edemaine edemaine merged commit 9b4acc9 into KaTeX:master May 12, 2021
KaTeX-bot added a commit that referenced this pull request May 12, 2021
@KaTeX-bot
chore(release): 0.13.10 [ci skip]
b03d0c2 
## [0.13.10](v0.13.9...v0.13.10) (2021-05-12)

### Bug Fixes

* Correct for negative margin in integrand lower limits ([#2987](#2987)) ([9b4acc9](9b4acc9))
@KaTeX-bot
Copy link
Member

🎉 This PR is included in version 0.13.10 🎉

The release is available on:

Your semantic-release bot 📦🚀

@edemaine edemaine mentioned this pull request Jun 1, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edemaine edemaine edemaine approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Wide indices of integrals with "\limits" overlap
4 participants
@ronkok @codecov-commenter @edemaine @KaTeX-bot
0