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

185847976 numberline point values #2229

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Copy link

cypress bot commented Mar 11, 2024

Passing run #11737 ↗︎

0 90 5 0 Flakiness 0

Details:

null
Project: collaborative-learning Commit: a65a914cc5
Status: Passed Duration: 11:18 💡
Started: Mar 19, 2024 10:56 PM Ended: Mar 19, 2024 11:07 PM

Review all test suite changes for PR #2229 ↗︎

Copy link

codecov bot commented Mar 11, 2024

Codecov Report

Attention: Patch coverage is 7.14286% with 52 lines in your changes are missing coverage. Please review.

Project coverage is 55.06%. Comparing base (39c3393) to head (32fbf0b).

❗ Current head 32fbf0b differs from pull request most recent head a65a914. Consider uploading reports for the commit a65a914 to get more accurate results

Files Patch % Lines
.../plugins/numberline/components/numberline-tile.tsx 0.00% 49 Missing ⚠️
...line/components/editable-numberline-min-or-max.tsx 25.00% 3 Missing ⚠️
Additional details and impacted files
@@                          Coverage Diff                          @@
##           187056474-numberline-open-points    #2229       +/-   ##
=====================================================================
- Coverage                             82.27%   55.06%   -27.21%     
=====================================================================
  Files                                   688      681        -7     
  Lines                                 34103    34056       -47     
  Branches                               8841     8832        -9     
=====================================================================
- Hits                                  28058    18754     -9304     
- Misses                                 5714    14569     +8855     
- Partials                                331      733      +402     
Flag Coverage Δ
cypress-regression ?
cypress-smoke 30.08% <7.14%> (-0.03%) ⬇️
jest 51.94% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dennisrcao dennisrcao marked this pull request as ready for review March 12, 2024 20:11
@dennisrcao dennisrcao changed the base branch from master to 187056474-numberline-open-points March 12, 2024 20:11
@dennisrcao
Copy link
Contributor Author

In case QA @johnTcrawford finds an issue where we create a point and then set the min greater than it, and the point persists off the numberline (when it should disappear) - then please ignore that edge case - we'll address that in a later ticket
https://www.pivotaltracker.com/n/projects/2441242/stories/187232408

Copy link
Member

@scytacki scytacki left a comment

Choose a reason for hiding this comment

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

looks good to me

@scytacki scytacki added the pending QA review A PR that is waiting for QA to review it before merging. label Mar 13, 2024
@scytacki scytacki requested a review from johnTcrawford March 13, 2024 21:49
@scytacki
Copy link
Member

There are cases where the point can't be moved so the value in label matches the tick mark that the point is on. For example the value might only show 5.99 or 6.01 and you can't move it to show 6.00. I've brought this up with Leslie. It isn't an easy problem to fix, so I wouldn't try unless it is required.

@scytacki
Copy link
Member

Leslie found an issue where the sparrows do not align with the points anymore. I'll work on fixing that later.

Copy link
Contributor

@johnTcrawford johnTcrawford left a comment

Choose a reason for hiding this comment

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

There is no Cypress code here (so far, at least), so I'm not sure why I was asked to review it. Is it because there is a plan to add Cypress code later, or was it just a mistake?

Base automatically changed from 187056474-numberline-open-points to master March 20, 2024 13:20
@scytacki scytacki removed their assignment Mar 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending QA review A PR that is waiting for QA to review it before merging. run regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants