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

fix: allow 0-width stroke width and 0-percent fill-opacity #73

Merged
merged 2 commits into from
Feb 4, 2022

Conversation

PierreSchwang
Copy link
Member

@PierreSchwang PierreSchwang commented Jan 26, 2022

Overview

Based on an issue on discord
grafik

Description

Now a 0-width stroke width and 0-percent fill-opacity is possible + code reformat

Submitter Checklist

  • Make sure you are opening from a topic branch (/feature/fix/docs/ branch (right side)) and not your main branch.
  • Ensure that the pull request title represents the desired changelog entry.
  • I tested my changes and approved their functionality.
  • I ensured my changes do not break other parts of the code
  • I read and followed the contribution guidelines

Maintainer checklist

Before the changes are marked as Ready for merge:

  • There are at least 2 approvals from project developers or maintainers for the pull request and no outstanding requests for change
  • Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
  • Changelog entries in the PR title are correct
  • Proper changelog labels are set so that the changelog can be generated automatically
  • If the change needs additional modification steps from users, e.g. for permission changes, make sure the entry is added to the pull request description so it can be appended to the changelog.

@PierreSchwang PierreSchwang requested a review from a team as a code owner January 26, 2022 19:00
@PierreSchwang
Copy link
Member Author

Regression I found out: Now the area is not clickable anymore, just the border, if the opacity is 0 no

@NotMyFault NotMyFault added Bugfix This PR fixes a bug ready for review and removed ready for review labels Jan 26, 2022
Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

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

My experience with Dynmap is limited. Is that something we can resolve on our end?

@PierreSchwang
Copy link
Member Author

My experience with Dynmap is limited. Is that something we can resolve on our end?

I've searched through dynmaps source as well, but I don't know either. I've created a separated issue as a feature request in dynmaps repo for that: webbukkit/dynmap#3637

@PierreSchwang
Copy link
Member Author

@NotMyFault Found a workaround by setting the color to #FF000000 when the opacity is set to 0. The opacity is then set back to 1 and an 0-alpha channel is passed to the fill color. therefor the fill is still being added to the dom, but it's invisible.

@NotMyFault NotMyFault requested a review from a team January 27, 2022 21:48
@NotMyFault
Copy link
Member

/rebase

Copy link
Member

@NotMyFault NotMyFault left a comment

Choose a reason for hiding this comment

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

Looks fine so far

@NotMyFault NotMyFault requested a review from a team January 31, 2022 18:15
@NotMyFault NotMyFault merged commit 69e1b86 into main Feb 4, 2022
@NotMyFault NotMyFault deleted the fix/allowZeroValues branch February 4, 2022 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix This PR fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants