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

Migrate to the latest version of canvas #758

Merged
merged 11 commits into from Jul 22, 2020

Conversation

vabarbosa
Copy link
Contributor

@vabarbosa vabarbosa commented Jul 15, 2020

Migrate to the latest canvas version

Changes

  • per @elyra/canvas release 7.0.5

    • Updated dependency react-intl (^3.0.0)
    • Added dependency react-redux (^7.0.0)
  • per @elyra/canvas release 8.0.1

    • Updated CSS class names and styling
    • Placed <CommonCanvas> inside a <IntlProvider>
    • Removed toolbarMenuActionHandler. Handling of these actions moved to the editActionHandler
    • Removed contextMenuActionHandler. Handling of these actions moved to the editActionHandler.
    • Renamed addComment action to createAutoComment
    • Renamed delete action to deleteSelectedObjects
  • per @elyra/canvas release 8.0.2

    • Added config createSupernode // user can disable creation of supernodes via configuration parameter
  • per @elyra/canvas release 8.0.4

    • Added config enableInsertNodeDroppedOnLink // user can drag nodes from the palette and drop them onto existing links in the flow which causes the node to be inserted between the two nodes joined by the link
  • Update screenshots in documentation

Fixes #645

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

Updated the canvas version and resolve breaking changes
@vabarbosa vabarbosa added component:pipeline-editor pipeline editor area:front-end status:Work in Progress Development in progress. A PR tagged with this label is not review ready unless stated otherwise. labels Jul 15, 2020
@vabarbosa vabarbosa self-assigned this Jul 15, 2020
@vabarbosa vabarbosa marked this pull request as draft July 15, 2020 15:40
@vabarbosa vabarbosa changed the title [WIP] Migrate to the latest version of canvas Migrate to the latest version of canvas Jul 15, 2020
@vabarbosa vabarbosa marked this pull request as ready for review July 15, 2020 22:25
@vabarbosa
Copy link
Contributor Author

vabarbosa commented Jul 15, 2020

all the major work is dont here and it is basically ready for reviews.

all that is left would be to merge with #744 and #752 (and resolve conflicts) once they are ready.

the second part of #750 (cardinality being deleted) cannot be confirmed as it was addressed in canvas yesterday and won't be available until next canvas release. the first part (incorrect cardinality) is addressed in #759

@vabarbosa vabarbosa removed the status:Work in Progress Development in progress. A PR tagged with this label is not review ready unless stated otherwise. label Jul 16, 2020
@marthacryan
Copy link
Member

marthacryan commented Jul 16, 2020

Just tried this out locally and it looks great! So sleek! For those who don't have time to check out the PR locally this is what the new canvas looks like:
image

As for #744, I think actually it might be a good idea to merge this PR first, because I think there's a handler in the newer version of canvas that would simplify that PR (beforeEditActionHandler).

We'll also need to update the pipeline documentation images to use the new interface.

@vabarbosa
Copy link
Contributor Author

Just tried this out locally and it looks great! So sleek! For those who don't have time to check out the PR locally this is what the new canvas looks like:

As for #744, I think actually it might be a good idea to merge this PR first, because I think there's a handler in the newer version of canvas that would simplify that PR (beforeEditActionHandler).

We'll also need to update the pipeline documentation images to use the new interface.

thanks for the reminder. i'll update to documentation images. this can be merged first if it is going to simplify #744

Replace the PIpeline Editor screenshots in docs to latest version
Copy link
Member

@karlaspuldaro karlaspuldaro left a comment

Choose a reason for hiding this comment

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

This is awesome! Works great.

We can create an issue to look at the paste button always enabled. On my last PR I thought it was behaving like that because I always had a node on the clipboard.

@vabarbosa vabarbosa marked this pull request as draft July 17, 2020 19:33
@vabarbosa vabarbosa marked this pull request as ready for review July 17, 2020 19:36
@vabarbosa
Copy link
Contributor Author

this PR is good to go for @elyra/canvas v8.0.32.
however, v9 has just been published. i will upgrade to v9 instead and resolve any issues that may arise.

@marthacryan
Copy link
Member

@vabarbosa Also, feel free to merge this before #752 merges as well and I can rebase. It seems like there's more to discuss on that PR than there is here.

@ajbozarth
Copy link
Member

I'll be reviewing this early this afternoon, will post code and UX reviews as I finish each

Copy link
Member

@ajbozarth ajbozarth left a comment

Choose a reason for hiding this comment

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

Code and UX LGTM but I found some UI issues on Safari (surprise surprise Safari again >.<). I've attached screen shots of the oddities below:
Screen Shot 2020-07-21 at 2 44 07 PM
Screen Shot 2020-07-21 at 2 44 24 PM
As you can see some of the borders and mouse over are missing or misaligned. No idea why it is like this on Safari and not Chrome.

@vabarbosa vabarbosa requested a review from ajbozarth July 21, 2020 22:52
@vabarbosa
Copy link
Contributor Author

Code and UX LGTM but I found some UI issues on Safari (surprise surprise Safari again >.<). I've attached screen shots of the oddities below:
As you can see some of the borders and mouse over are missing or misaligned. No idea why it is like this on Safari and not Chrome.

thanks, Safari discrepancies should be resolved.

@lresende lresende merged commit 9021f9a into elyra-ai:master Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate to latest Elyra Canvas release
5 participants