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

188651857 v3 DI Top Level Graph Features #1686

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

tealefristoe
Copy link
Contributor

@tealefristoe tealefristoe commented Dec 13, 2024

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

This PR adds support for many top level graph features to the API. The following features are all supported in create, get, and update requests:

  • backgroundColor
  • displayOnlySelectedCases
  • filterFormula
  • hiddenCases
  • pointColor
  • pointConfig ("points", "bars", "bins", etc)
  • pointsFusedIntoBars
  • pointSize
  • showMeasuresForSelection
  • strokeColor
  • strokeSameAsFill
  • transparent (doesn't seem to do anything)

@tealefristoe tealefristoe marked this pull request as draft December 13, 2024 17:23
@tealefristoe tealefristoe added the v3 CODAP v3 label Dec 13, 2024
Copy link

codecov bot commented Dec 13, 2024

Codecov Report

Attention: Patch coverage is 97.36842% with 1 line in your changes missing coverage. Please review.

Project coverage is 68.31%. Comparing base (6c2e921) to head (07d1667).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
v3/src/components/graph/graph-component-handler.ts 97.36% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (6c2e921) and HEAD (07d1667). Click for more details.

HEAD has 9 uploads less than BASE
Flag BASE (6c2e921) HEAD (07d1667)
cypress 10 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1686       +/-   ##
===========================================
- Coverage   85.78%   68.31%   -17.48%     
===========================================
  Files         607      607               
  Lines       31005    31029       +24     
  Branches     8586     8007      -579     
===========================================
- Hits        26599    21198     -5401     
- Misses       4251     9676     +5425     
  Partials      155      155               
Flag Coverage Δ
cypress 43.65% <0.00%> (-31.10%) ⬇️
jest 53.69% <97.36%> (+0.03%) ⬆️

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 Dec 13, 2024

codap-v3    Run #5547

Run Properties:  status check passed Passed #5547  •  git commit 07d1667f0d: Merge main.
Project codap-v3
Branch Review 188651857-v3-di-top-level-graph-features
Run status status check passed Passed #5547
Run duration 01m 50s
Commit git commit 07d1667f0d: Merge main.
Committer Teale Fristoe
View all properties for this run ↗︎

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

@tealefristoe
Copy link
Contributor Author

@bfinzer @kswenson
This PR is pretty close to being done. I still need to add tests, but before I do I have several questions that I want to ask:

  • Are there any other graph features that should be included? (See the PR description.)
  • Do we want to include all of the listed features? Should any be in update but not create?
  • Should any feature names be changed?
  • There are many different graph types, and not all of these features make sense for every graph type. I'm not doing any compatibility or error checking when assigning the features, I'm just doing whatever the API request asks. Is this ok? I'm guessing it would be pretty difficult and time consuming to thoroughly check everything.
  • Some of these features don't work when creating a graph. In particular, bar charts don't work in fresh graphs; it seems like you need to transition a point graph into a bar graph for them to work properly. What should I do about this? I could remove these as options from the create API request. Or I could fix the graph to make bar charts work immediately, though I'm guessing it won't be easy. My inclination is to leave the features in place and write a PT story about fixing newly created bar graphs to deal with down the line.

@bfinzer
Copy link
Contributor

bfinzer commented Dec 13, 2024

  • We don't need to support any additional graph features at this time.
  • I think it's fine to include all the listed features.
  • I think the feature names are fine as you've got them.
  • We don't need error checking, at least for now.
  • Writing the story for creating a bar chart is a good idea. We don't need this feature to work for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3 CODAP v3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants