-
Notifications
You must be signed in to change notification settings - Fork 1
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: compliance rules highlight activities on popover click #85
Conversation
♻️ PR Preview b557648 has been successfully destroyed since this PR has been closed. 🤖 By surge-preview |
src/compliance-rules.js
Outdated
tippyInstance.popper.addEventListener("mouseover", (event) => { | ||
if (event.target.nodeName === "TD") { | ||
const selectedRow = event.target.parentElement; | ||
selectedRow.style.backgroundColor = "lightgray"; | ||
const activityName = selectedRow.firstElementChild.textContent; | ||
const activityId = getElementIdByName(activityName) | ||
//highlight activity | ||
bpmnVisualization.bpmnElementsRegistry.addCssClasses(activityId, "cause-violation") | ||
// workaround for https://github.com/process-analytics/icpm-demo-2022/issues/87 | ||
if(complianceData.has(activityName)){ | ||
addRippleCircles(activityId, bpmnVisualization) | ||
} | ||
} | ||
}); | ||
|
||
tippyInstance.popper.addEventListener("mouseout", (event) => { | ||
if (event.target.nodeName === "TD") { | ||
const selectedRow = event.target.parentElement; | ||
selectedRow.style.backgroundColor = ""; | ||
const activityName = selectedRow.firstElementChild.textContent; | ||
const activityId = getElementIdByName(activityName) | ||
//highlight activity | ||
bpmnVisualization.bpmnElementsRegistry.removeCssClasses(activityId, "cause-violation") | ||
// workaround for https://github.com/process-analytics/icpm-demo-2022/issues/87 | ||
if(complianceData.has(activityName)){ | ||
addRippleCircles(activityId, bpmnVisualization) | ||
} | ||
} | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: the highlight of the rows of the popover should be done with CSS directly, as this is done in https://github.com/process-analytics/bonita-day-demo-2023/pull/10/files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@assynour did you have time to check the review?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix it this week
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 57b6053
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Design and refactoring to plan
src/compliance-rules.js
Outdated
tippyInstance.popper.addEventListener("mouseover", (event) => { | ||
if (event.target.nodeName === "TD") { | ||
const selectedRow = event.target.parentElement; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: process-analytics/bonita-day-demo-2023#18 registers the event listener directly in the elements of the HTML of the popover. This is easy to register them directly on the rows (TR elements).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will take this into account
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in 6e0c300
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I didn't check the code in details, but it does the job and reuse what has been done in the bonita-day demo
✔️ Tested with the surge preview environment
closes #62