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

Double size of remaining components of logic editor #659

Merged
merged 2 commits into from
Jan 18, 2025

Conversation

Amadeus1791
Copy link
Contributor

@azoitl
Copy link
Contributor

azoitl commented Jan 13, 2025

It looks like the changes in the LED led to some issues with grid alignment.

@ptziegler
Copy link
Contributor

ptziegler commented Jan 13, 2025

It looks like the changes in the LED led to some issues with grid alignment.

The "Snap to Grid" is working. It's just that the LED is not where it's supposed to be.

UIThreadRunnable.syncExec(() -> {
editor.drag(editPart, 200, 45);
forceUpdate(editor.getSWTBotGefViewer());
});

The label snaps to the circuit, not to the connectors. Because the figure is now twice as large, the circuit starts at y = 52, rather than y = 50. And as a result, the position where the label is moved to is too far away for the "snap to grid" to be applied.

image

@ptziegler
Copy link
Contributor

Just a thought: If the size of the figures are doubled, then having a threshhold of only 5 pixels for the snap-to-grid is really low. Would it make sense to double that as well?

@Amadeus1791
Copy link
Contributor Author

Amadeus1791 commented Jan 18, 2025

Thanks for you feedback, I appreciate it a lot! I added a follow-up commit to fix grid and snap-to-geometry tests. Those two test were affected by the component size changes. It happened since the snap points shifted relative to the grid due to the changed component boundaries.

@azoitl azoitl merged commit b5352f2 into eclipse-gef:master Jan 18, 2025
14 checks passed
@ptziegler ptziegler added this to the 3.23.0 milestone Jan 19, 2025
@ptziegler
Copy link
Contributor

@azoitl I think for the future, it would be better to just squash multiple commits. Or is there any benefit in keeping them separate?

image

@azoitl
Copy link
Contributor

azoitl commented Jan 19, 2025

In this specific case I was not sure. Normally I do not squash if each of the commits is by itself an own independent contribution. I kept it separate because I didn't want to loos the information that changing sizes may have other impact. Not sure if that was good.

@ptziegler
Copy link
Contributor

This itself is fine, but I usually like to avoid doing a "rebase and merge" when there are multiple commits. Otherwise it's simply not obvious that both commits are related, if one would ever need to revert them.

@azoitl
Copy link
Contributor

azoitl commented Jan 19, 2025

@ptziegler that is a good point. Will keep that in mind.

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

Successfully merging this pull request may close these issues.

4 participants