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

Fix Link Action to accept the right input for validation #4696

Merged
merged 1 commit into from
Jan 7, 2022
Merged

Fix Link Action to accept the right input for validation #4696

merged 1 commit into from
Jan 7, 2022

Conversation

shefalijoshi
Copy link
Contributor

@shefalijoshi shefalijoshi commented Jan 7, 2022 •
edited
Loading

Resolves #4695

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

  • Changes address original issue?
  • Unit tests included and/or updated with changes?
  • Command line build passes?
  • Has this been smoke tested?
  • Testing instructions included in associated issue?

@codecov
Copy link

codecov bot commented Jan 7, 2022

Codecov Report

❗ No coverage uploaded for pull request base (release/1.8.3@187da3c). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##             release/1.8.3    #4696   +/-   ##
================================================
  Coverage                 ?   56.29%           
================================================
  Files                    ?      718           
  Lines                    ?    22785           
  Branches                 ?     1656           
================================================
  Hits                     ?    12827           
  Misses                   ?     9551           
  Partials                 ?      407

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 187da3c...b01651b. Read the comment docs.

khalidadil
khalidadil approved these changes Jan 7, 2022
View reviewed changes
Copy link
Contributor

@khalidadil khalidadil left a comment

Choose a reason for hiding this comment

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

Looks ok to me, but I'm not sure why there were 2 params there

@shefalijoshi
Copy link
Contributor Author

That was legacy code that was ported to es6. Now that we have this.object we don't need the object to be passed in anymore.

@shefalijoshi shefalijoshi merged commit 8314d03 into release/1.8.3 Jan 7, 2022
@shefalijoshi shefalijoshi deleted the fix-links branch January 7, 2022 22:46
@khalidadil khalidadil linked an issue Jan 7, 2022 that may be closed by this pull request
Unable to create links for objects #4695
Closed
5 tasks
unlikelyzero pushed a commit that referenced this pull request Jan 20, 2022
@shefalijoshi @davetsay
@nikhilmandlik @michaelrogers
release 1.8.3 - master (#4752)
dcaad40 
* Fix object creation (#4675)

* Save the object before adding it to the parent so that transaction committing works properly
* Fix object creation - composition policy changes

* use relative path (#4683)

* Notebook Snapshotting to the default Notebook isn't working (#4475)

* Notebook Snapshotting to the default Notebook isn't working #4469

* Check for ref when trying to get the style receiver. (#4687)

Also ensure that the property being displayed in Properties is actually a part of the domainObject.

* Mct 4555 rebased v3 (#4689)

* Preserve the previousFocusedImage for subscription updates, bound change for local and fixed time

* Only preserve previous focused image if paused

* Forcibly reset imageContainer size to prevent aspect ratio distortion

* Remove unneccesary mixin invocation

* Use image history instead of imagehistory size for watcher. Revert other changes

* Added check if last image index is selected

* isPaused instead of paused

Co-authored-by: Michael Rogers <[email protected]>

* Snapshot notice link not navigating as expected #4194 (#4686)

* Fix Link Action to accept the right input for validation (#4696)

* Remove snapshot from release version (#4721)

Co-authored-by: David Tsay <[email protected]>
Co-authored-by: Nikhil <[email protected]>
Co-authored-by: Michael Rogers <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@khalidadil khalidadil khalidadil approved these changes

@nikhilmandlik nikhilmandlik Awaiting requested review from nikhilmandlik

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

Successfully merging this pull request may close these issues.

Unable to create links for objects
2 participants
@shefalijoshi @khalidadil