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

Empty line at the end of description didn't vanish after deletion #763

Closed
ingorichtsmeier opened this issue Feb 7, 2018 · 6 comments
Closed
Labels
bug Something isn't working good first issue Good for newcomers modeling
Milestone

Comments

@ingorichtsmeier
Copy link

I add a newline at the end of a task name.

grafik

Then I delete the the empty line.

Expected Behavior

The task name is in the middle of the task

Actual Behavior

The task name is shift to the top

grafik

Steps to reproduce the Behavior

  1. Add a newline with shift+enter at the end of a task name.
  2. Click to the next task.
  3. Delete the empty line
@nikku nikku added bug Something isn't working modeling labels Feb 8, 2018
@nikku nikku added this to the Backlog milestone Feb 8, 2018
@nikku nikku added the good first issue Good for newcomers label Feb 8, 2018
@nikku
Copy link
Member

nikku commented Feb 8, 2018

I can confirm this bug. Thanks for the report.

@nikku nikku added the backlog Queued in backlog label Feb 14, 2018
@nikku nikku removed this from the Backlog milestone Feb 14, 2018
@gustavjf
Copy link
Contributor

I think this might be a bug in diagram-js instead of bpmn-js.

Two newlines are inserted when pressing shift+enter in a contenteditable div, which are later parsed as <tspan> tags when pressing enter.

Before pressing enter:
bpmnjs-screenshot_02

After pressing enter:
bpmnjs_screenshot_04

Once those two characters have been inserted and the task is rendered as SVG after pressing enter, only one is removed on backspace when trying to edit again. This leaves a stray <tspan> in place, causing the unwanted space.

bpmnjs-screenshot_03

I thought of trimming whitespace before parsing as <tspan> tags here but I noticed two things:

  1. A user will not see the rendered effect of inserting newlines without any text following it. This may or may not be desirable.

  2. The text content of the contenteditable div and the result of the rendered SVG will not be in sync. The rendered SVG will not have trailing <tspan>s but the contenteditable div will retain the newline characters when going back to edit.

For what it's worth, trimming and thus preventing trailing whitespace from being rendered as SVG would also prevent the task's text from being rendered outside of the task itself:

bpmnjs-screenshot_01

Any thoughts? If the two points above aren't an issue, I could prepare a PR with the trim I mentioned.

@nikku
Copy link
Member

nikku commented Mar 6, 2019

My guess is that we must properly handle this issue in diagram-js-direct-editing.

One approach would be to make sure that the text is trimmed after editing, committing it without trailing and leading whitespace.

@gustavjf
Copy link
Contributor

Makes sense, how about this?

merge-me bot pushed a commit to bpmn-io/diagram-js-direct-editing that referenced this issue Mar 25, 2019
@nikku
Copy link
Member

nikku commented Mar 25, 2019

Needs patch release of diagram-js.

@gustavjf
Copy link
Contributor

gustavjf commented Apr 1, 2019

Fixed via 0cfd960

@gustavjf gustavjf closed this as completed Apr 1, 2019
@ghost ghost removed the backlog Queued in backlog label Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers modeling
Projects
None yet
Development

No branches or pull requests

3 participants