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.

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

Add 'get' canvas functions to ranges #79

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

Add 'get' canvas functions to ranges #79

wants to merge 4 commits into from

Conversation

MImranAsghar
Copy link

  • Add getCanvases, getCanvasById, getCanvasByIndex functions for the range type

@MImranAsghar MImranAsghar mentioned this pull request Nov 23, 2020
Open
@mejackreed mejackreed mentioned this pull request Dec 4, 2020
Open
@stephenwf
Copy link
Member

I'm not sure why, but 3 tests appear to be failing now:

195 passing (2s)
  3 failing
  1) pres3 presentation 3 range has a canvas:
     TypeError: Cannot read property '__jsonld' of undefined
      at context.<anonymous> (test/tests/pres3.js:48:96)
  2) pres3 presentation 3 has an annotation body:
     TypeError: Cannot read property 'getContent' of undefined
      at context.<anonymous> (test/tests/pres3.js:54:26)
  3) pres3 presentation 3 has a label:
     TypeError: Cannot read property 'getLabel' of undefined
      at context.<anonymous> (test/tests/pres3.js:62:26)

@stephenwf stephenwf mentioned this pull request Jan 22, 2021
Open
@MImranAsghar
Copy link
Author

Ah interesting, looking into it 👍

@MImranAsghar
Copy link
Author

Hi @stephenwf , fixed the tests

@MImranAsghar
Create canvas functions for range type
b9136d9 
- Create getCanvases, getCanvasById, getCanvasByIndex functions for the range type
@MImranAsghar
Move getCanvasById function from ...
45f71e7 
sequence to range as well
- Add test for getCanvasById range
@MImranAsghar
Update getCanvases to assign correct ...
d0a54a6 
... index to each canvas in range
@MImranAsghar
Add functionality to handle fragments in ...
53a2e81 
... ranges when getting canvases
@MImranAsghar
Copy link
Author

Hi @stephenwf , I was wondering if you got a chance to take a look at the changes. do you think this can be merged? 🤔

@stephenwf
Copy link
Member

Sorry yes I think so 👍 @edsilv ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MImranAsghar @stephenwf