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

Simple marching ants with clean SCSS styling #266

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

simonbethke
Copy link
Contributor

@simonbethke simonbethke commented Nov 6, 2024

I implemented simple marching ants without introducing complexity.

marching-ants2.mp4

src/ui/scss/style.scss Outdated Show resolved Hide resolved
src/ui/scss/style.scss Outdated Show resolved Hide resolved
@willeastcott
Copy link
Contributor

Would you be able to add a little gif/video of the effect to the PR description please?

@simonbethke
Copy link
Contributor Author

Just added it to the description.

Copy link
Contributor

@willeastcott willeastcott left a comment

Choose a reason for hiding this comment

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

This is great! 🐜🐜🐜🐜🐜

Copy link
Member

@slimbuck slimbuck left a comment

Choose a reason for hiding this comment

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

This is pretty great, thanks @simonbethke.

A few things:

  • the brush tool outline is broken (I can't see why from a quick look at your changes)
  • I'm not sure about the 2px line width... that combined with the animating ants looks a bit ... loud to me. I can be persuaded though, will chat with @willeastcott tomorrow.

@@ -16,9 +16,6 @@ class BrushSelection {
const circle = document.createElementNS(svg.namespaceURI, 'circle') as SVGCircleElement;
circle.setAttribute('r', radius.toString());
circle.setAttribute('fill', 'rgba(255, 102, 0, 0.2)');
circle.setAttribute('stroke', '#f60');
Copy link
Member

Choose a reason for hiding this comment

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

I think you meant to remove the previous line too

@simonbethke
Copy link
Contributor Author

simonbethke commented Nov 7, 2024

Hm, the brush outline isn't broken. I am just not used to moving single atoms one by one, so I changed it to something that appears correct to me.
I am used to either having much more communication or being free to do things the way that seems to be correct to me.

The 2px width is just a poor try to raise the visibility. What would actually be needed is to fill the gaps in the dashed line with something darker. But with svg this is only possible by duplicating the entire polyline

@simonbethke
Copy link
Contributor Author

I have changed the 2px width of the lines to 1px and also reverted the brush-outline to dashed again.
Updated the video in the opening post.

@simonbethke
Copy link
Contributor Author

Do you have any auto-formatting setup that I can use in vscode? I keep seeing reformattings :)

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.

3 participants