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

feat: add RPA editor to Camunda Modeler #4807

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Conversation

marstamm
Copy link
Member

@marstamm marstamm commented Jan 23, 2025

Proposed Changes

The source code for the RPA worker can be found here: https://github.com/camunda/rpa-frontend

This PR integrates the RPA Editor UI into the Camunda Modeler.

Core concepts:

  • Connection to the RPA worker is managed by the Editor instance
  • @camunda/rpa-integration provides React components for the testing UI, they use the editor instance (required prop) to communicate.

Happy to give sync walkthrough

screencap-2025-01-23-18-04-17.webm

What will change (UI/Interactions):

  • Links to docs
  • Library Suggestions
  • (potentially) deployment to local instance only

RPA worker setup

To test against a local worker:

  1. Download the RPA worker from github
  2. Add a rpa-worker.properties in the same directory as the rpa worker with the content:
    camunda.rpa.python.extra-requirements=extra-requirements.txt
    camunda.client.zeebe.enabled=false
    
  3. Add a extra-requirements.txt in the same directory as the rpa worker with the content:
    camunda-utils @ https://github.com/camunda/rpa-python-libraries/releases/download/camunda-utils-v0.1.0a0/camunda_utils-0.1.0a0-py3-none-any.whl
    camunda-rpa @ https://github.com/camunda/rpa-python-libraries/releases/download/camunda-rpa-v0.1.0a0/camunda_rpa-0.1.0a0-py3-none-any.whl
    
    This step will not be required in the 1.0 release
  4. Start the RPA worker by running the executable

Checklist

To ensure you provided everything we need to look at your PR:

  • Brief textual description of the changes present
  • Visual demo attached
  • Steps to try out present, ==> see comment
  • Related issue linked via Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label Jan 23, 2025
@nikku
Copy link
Member

nikku commented Jan 24, 2025

@marstamm Could you share run instructions for non-developers?

@marstamm
Copy link
Member Author

Copy link
Collaborator

@barmac barmac left a comment

Choose a reason for hiding this comment

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

I am not able to give a full review right now, but let's sync on this later on.

Comment on lines +670 to +675
resource.form = contents;
} else {

// Fallback for unknown resource, cf.
// https://github.com/camunda-community-hub/zeebe-client-node-js/blob/7969ce1808c96a87519cb1a3f279287f30637c4b/src/zb/ZBClient.ts#L873-L886

Copy link
Collaborator

Choose a reason for hiding this comment

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

Given that we are migrating to the SDK, we may want to adjust this part
#4817

client/package.json Outdated Show resolved Hide resolved
client/package.json Outdated Show resolved Hide resolved
@barmac
Copy link
Collaborator

barmac commented Feb 4, 2025

Suggestion: reorder buttons to make logical order:

Screen.Recording.2025-02-04.at.16.49.04.mov

@barmac
Copy link
Collaborator

barmac commented Feb 4, 2025

Suggestion: Use a single field for the URL

image

@barmac
Copy link
Collaborator

barmac commented Feb 4, 2025

Suggestion: scroll only to the bottom

image

@barmac
Copy link
Collaborator

barmac commented Feb 4, 2025

Question: Can we use Codemirror like we do everywhere in the app?

@barmac
Copy link
Collaborator

barmac commented Feb 4, 2025

To fix: Scrolling

Screen.Recording.2025-02-04.at.16.54.29.mov


describe('#render', function() {

it('should render with NO xml', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

JSON?

@barmac
Copy link
Collaborator

barmac commented Feb 4, 2025

@marstamm to add a file which we can run with the docker worker.
Also we need a macos image.

@barmac
Copy link
Collaborator

barmac commented Feb 4, 2025

@philippfromme @marstamm and I looked at the PR, and tested the solution in action. View our remarks above.
@marstamm's introduction was recorded and can be accessed at https://camunda.slack.com/archives/C032H77434N/p1738685029197899?thread_ts=1737736287.487929&cid=C032H77434N

marstamm added a commit that referenced this pull request Feb 4, 2025
@marstamm
Copy link
Member Author

marstamm commented Feb 4, 2025

Suggestion: reorder buttons to make logical order:
Suggestion: Use a single field for the URL

@PhilippaC22 , can you have a look and give a 👍 from design?

Suggestion: scroll only to the bottom

Done 1205293

Question: Can we use Codemirror like we do everywhere in the app?

Not in this iteration.

@marstamm to add a file which we can run with the docker worker.

The Complete .rpa file:
initial.rpa.txt

@marstamm
Copy link
Member Author

marstamm commented Feb 4, 2025

To fix: Scrolling

This is caused by the Iframe URL being unreachable. This will display the run log with the production Worker

@marstamm
Copy link
Member Author

This is caused by the Iframe URL being unreachable. This will display the run log with the production Worker

This is now fixed. I added a new getting started guide in the main body that used the latest RPA worker, please try it out :)

@nikku nikku requested a review from lmbateman February 11, 2025 08:57
@nikku
Copy link
Member

nikku commented Feb 11, 2025

I gave this a quick review. The current integration (testing, deploy button) looks a little bit like an alien to me, it does not follow the Desktop modeler UI.

Did we/do we consider sensible adjustments to make it feel more native, i.e. with theeming (#4511)? This is something that the core team could follow-up on.

CC @lmbateman @barmac, let's ensure this integrates nicely enough for the users, or decide how/when we get to a "nicely enough" integration.

@nikku
Copy link
Member

nikku commented Feb 11, 2025

@marstamm Is there a reason this is still in draft? What kind of additional feedback do you seek before moving it to ready for review, when do you want to see this merged?

@marstamm marstamm marked this pull request as ready for review February 11, 2025 09:07
@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Feb 11, 2025
@bpmn-io-tasks bpmn-io-tasks bot removed the in progress Currently worked on label Feb 11, 2025
@marstamm
Copy link
Member Author

We still need to define how we want to ship this before the 8.7.0 release, e.g. release behind a feature flag for the alphas

@marstamm
Copy link
Member Author

Did we/do we consider sensible adjustments to make it feel more native, i.e. with theeming (#4511)? This is something that the core team could follow-up on.

The Components are designed to be used by both desktop and web modeler, to avoid maintaining multiple implementations.

The Carbon styles are taken from the parent application. Currently, these are just the default carbon styles, but can be changed if desired:

@import (css) '@carbon/styles/css/styles.css';

@lmbateman
Copy link

@abdul99ahad This might be your first Carbon-theming project. :) Here are my initial thoughts:

  • Empty state
    • Can we hide the graphic in the Desktop Modeler?
    • The font size is 16px; in the Variables tab empty state, the font size is 14px.
    • Can we restyle the button and link to Desktop Modeler standard?
  • Footer
    • I’m not sure what the new icon is supposed to be - a test tube? There are a few "test" icons in Carbon, but I'm not sure any of them really convey the idea well (or translate to the Desktop Modeler).
    • The “not connected” icon is the wrong color; I think it’s #ff0000, and should be #cc0000.
  • Welcome popup
    • The popup is very wide. Not sure if this is a real issue, but it doesn’t fit with our other popups. Also, typographical principles state that a line should be between 40 and 80 characters long. The longest line is just over 80 characters, so we may want to reduce the width a bit for that reason.
    • Additional configurations accordion
      • The accordion is a new component for the Desktop Modeler. I think it's the best component for progressive disclosure, but I wonder if we can just show the Base URL field and state that it's optional, since it's only a single field. The accordion feels a bit like overkill.
      • When it’s focused, the accordion looks like a dropdown; I’m not sure there’s anything we can do here aside from not focusing it, which seems suboptimal.
      • When it’s not focused, the accordion does look a bit alien. If we keep it, can we style it a bit in the Desktop Modeler? Darkening the borders might help; it looks like the current color is #e0e0e0, and the borders in the properties panel are #b9bcc6, so significantly darker.
      • Base URL text field: Can we style it like the Properties panel input fields? I see differences in the border color and width (on three sides); the background color (slightly); and the border radius.

I couldn’t figure out how to progress past this point.

@lmbateman
Copy link

Just to clarify my previous comment: My questions about changes were for the Desktop Modeler team. We can hopefully make all the changes I mentioned using a theme that won't affect the styling in Web Modeler. I don't expect the RPA team to make these changes. cc: @PhilippaC22

@marstamm
Copy link
Member Author

marstamm commented Feb 12, 2025

I think most of the things you mentioned can be fixed with some CSS.

The accordion feels a bit like overkill.

That is something we need to fix in the RPA Component instead of the modeler. I'll see if I have time this week. We can also push UI fixes later, as long as the base functionality is not broken

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs review Review pending
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants