Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Class Diagram: Adjust spacing of newly added lines to a class #357

Merged
merged 2 commits into from
Aug 9, 2024

Conversation

farisd16
Copy link
Member

@farisd16 farisd16 commented Jun 9, 2024

Checklist

  • [ ] I documented the TypeScript code using JSDoc style.
  • I added multiple screenshots/screencasts of my UI changes
  • [ ] I translated all the newly inserted strings into German and English

Motivation and Context

Fixes #333

Description

I have adjusted the height of newly added lines to a class to match the height of lines from the preview (specifically class-preview.ts)

Steps for Testing

  1. Create a class diagram
  2. Add a new class
  3. Add new lines (attribute, method, etc.) to the class
  4. Assert that all lines have even spacing

Test Coverage

| File | Branch | Line |
| uml-classifier-member.ts | 100% | 100% |

Screenshots

Before:
image

After:
image

@farisd16 farisd16 added the bug Something isn't working label Jun 9, 2024
@farisd16 farisd16 changed the title Class Diagram: Adjust spacing of newly added lines to UML elements Class Diagram: Adjust spacing of newly added lines to a class Jun 9, 2024
@farisd16 farisd16 requested a review from FelixTJDietrich June 13, 2024 12:44
@FelixTJDietrich
Copy link
Contributor

@farisd16 it is maybe a bit too much space now? Can you make it similar to Constant 4, Constant 5, Constant 6 in the before screenshot?

@farisd16
Copy link
Member Author

@farisd16 it is maybe a bit too much space now? Can you make it similar to Constant 4, Constant 5, Constant 6 in the before screenshot?

Sure, this is what it looks like:

image

@FelixTJDietrich
Copy link
Contributor

Somehow the vertical padding is now smaller, don't you think?

Copy link
Contributor

@FelixTJDietrich FelixTJDietrich left a comment

Choose a reason for hiding this comment

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

Code changes look good to me and I could verify that the spacing looks correct now locally :)
image

@loreanvictor
Copy link
Contributor

code looks good to me too, though I wonder if we need to manage these visual constants a bit better (perhaps in a follow-up issue).

@farisd16 farisd16 self-assigned this Jul 9, 2024
@FelixTJDietrich FelixTJDietrich added the ready for merge pr is ready to bemerged label Aug 8, 2024
@krusche krusche merged commit 38209a3 into develop Aug 9, 2024
4 checks passed
@krusche krusche deleted the bugfix/uml-element-spacing branch August 9, 2024 07:31
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working ready for merge pr is ready to bemerged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weird spacing in classes, enums between lines
4 participants