-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
fixed 1px shift when hovering card over holder #1921
fixed 1px shift when hovering card over holder #1921
Conversation
PR-SERVER-BOT: You can play around with it here: https://test.virtualtabletop.io/PR-1921/pr-test (or any other room on that server) After merging, a backup will be available at |
this is gonna be problematic as the more specific properties have higher priorities |
The single biggest issue was the |
I was tempted to change the calculated width to be a plain 1px but decided against it since it's now easily overridden and someone, somewhere may depend on that. |
yeah but this would affect anyone who only changed the general border property |
This is very bizarre to me: |
yeah this is pointless. would agree on removing |
This fixes, for example, dominoes where all of the dominoes are dropped into a giant holder. They all shift by 1px up and left whenever you are dragging one over the play area. But it does not fix the 1px shift that happens to the individual domino that you are dropping. I don't know if that is fixable or not. Either way, this is already a lot better. |
Won't this or break every game where someone used "border" to define "border-width" or "border-style"? |
No. The border shorthand will override the individual declarations. |
border-color: #d8d8d8 #ccc #ccc #d8d8d8; | ||
} | ||
|
||
.holder > .widget, .widget.droppable.droptarget.holder.transparent > .widget { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this looks confusing. Needs testing if this breaks the border on transparent holders.
I remembered it the other way around. I'll trust you until I can test it. Damn it's been just a couple of months away from css... |
Using What this does change, I think, is that holders without borders used to show a highlight when dropping but after this PR they wouldn't. I didn't test that though. |
Transparent holders behave the same. But this statement is correct. Something like "border":"none" on a holder in production shows an outline when dropping. There is no outline in this PR. Some game designers might rely on the existing behavior to indicate where the holders are. |
Probably worth it though. |
If they used the transparent class, the borders still show up when dropping on it. That's one of the changes here. Transparent holders still have borders, but they are just set to 'transparent'. If they remove the border, I don't know how to help that. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that even if a couple of games might rely on the existing behavior, way more than that would benefit from the annoying 1px shift. And if the designer was smart enough to make css that would break this, then they are probably smart enough to fix it. Or use the Discord help channel. Approved through test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes a shift in Expeditions and it doesn't seem to break The Team. Didn't test anything else, I think.
And I totally agree that it should have been like this from the start.
This update breaks the holder CSS apart a little so that when a border-width or other element is set, shifting does not occur in the contents.