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

188363563 Formula Editor API Requests #1560

Merged
merged 4 commits into from
Oct 25, 2024

Conversation

tealefristoe
Copy link
Contributor

@tealefristoe tealefristoe commented Oct 24, 2024

PT Story: https://www.pivotaltracker.com/story/show/188363563

This PR adds support for the notify attribute with request: formulaEditor API request, which displays the formula editor from a plugin.

This required more work than expected. The main problem is that, previously, each table and case card had a separate EditFormulaModal for each attribute. This was awkward for a couple of reasons: the API handler had to use another tile's modal, and if there was no table or case card associated with the given attribute, there was no modal to show at all. To fix this issue, a single modal was added to the Container, which any tile can use for any attribute via uiState.editFormulaAttributeId.

  • The table and case card tiles use this new method, which simplifies both and reduces the total number of components.
  • The EditFormulaModal is no longer wrapped in a DataSetContext, so it needs to find the dataset that contains the specified attribute, and then provides its own DataSetContext to its children.
  • EditFormulaModal has been moved from case-tile-common to common, since it is no longer tied to tables and case cards.
  • The only other minor change to EditFormulaModal is that the useEffect that refreshes the display formula now triggers when the attribute changes.

This work can be tested with this Codap link: https://codap3.concord.org/branch/v3-di-formula-editor-request/#file=examples:Mammals
And this Multidata link: https://models-resources.concord.org/multidata-plugin/branch/display-formula-editor/index.html

@tealefristoe tealefristoe marked this pull request as draft October 24, 2024 20:39
@tealefristoe tealefristoe added the v3 CODAP v3 label Oct 24, 2024
Copy link

codecov bot commented Oct 24, 2024

Codecov Report

Attention: Patch coverage is 94.87179% with 4 lines in your changes missing coverage. Please review.

Project coverage is 84.34%. Comparing base (cdd1b3a) to head (f3ccf4c).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
...src/data-interactive/handlers/attribute-handler.ts 25.00% 3 Missing ⚠️
v3/src/components/case-card/case-card.v2.js 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1560      +/-   ##
==========================================
+ Coverage   84.33%   84.34%   +0.01%     
==========================================
  Files         587      587              
  Lines       29606    29609       +3     
  Branches     8124     8125       +1     
==========================================
+ Hits        24967    24973       +6     
- Misses       4305     4481     +176     
+ Partials      334      155     -179     
Flag Coverage Δ
cypress 73.80% <94.44%> (-0.01%) ⬇️
jest 52.45% <34.61%> (-0.08%) ⬇️

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.

Copy link

cypress bot commented Oct 24, 2024

codap-v3    Run #4755

Run Properties:  status check passed Passed #4755  •  git commit a63aeb2510: 188363563 Formula Editor API Requests (#1560)
Project codap-v3
Branch Review main
Run status status check passed Passed #4755
Run duration 08m 44s
Commit git commit a63aeb2510: 188363563 Formula Editor API Requests (#1560)
Committer Teale Fristoe
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 1
Tests that did not run due to a developer annotating a test with .skip  Pending 45
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 218
View all changes introduced in this branch ↗︎

Copy link
Member

@kswenson kswenson 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 -- nice refactor!

@tealefristoe tealefristoe merged commit a63aeb2 into main Oct 25, 2024
18 checks passed
@tealefristoe tealefristoe deleted the 188363563-v3-di-formula-editor-request branch October 25, 2024 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants