Page MenuHomePhabricator

[web] [fix] [ENG-1005] arrows visually broken on safari
AbandonedPublic

Authored by benschac on Apr 14 2022, 7:54 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 19, 5:58 PM
Unknown Object (File)
Tue, Nov 19, 5:58 PM
Unknown Object (File)
Tue, Nov 19, 5:55 PM
Unknown Object (File)
Fri, Nov 15, 9:06 AM
Unknown Object (File)
Tue, Nov 12, 3:39 PM
Unknown Object (File)
Oct 17 2024, 8:02 PM
Unknown Object (File)
Oct 17 2024, 8:02 PM
Unknown Object (File)
Oct 17 2024, 7:58 PM

Details

Summary

arrows on safari are visually broken. add transparent color.

https://linear.app/comm/issue/ENG-1005/arrows-broken-in-safari

before:

Image 2022-04-14 at 10.55.09 AM.jpg (320×838 px, 18 KB)

after:

Image 2022-04-14 at 10.55.23 AM.jpg (322×858 px, 17 KB)

Test Plan

go to /comm/ in safari. Make sure the arrows match chrome.

Diff Detail

Repository
rCOMM Comm
Branch
fix-arrows
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

benschac edited the summary of this revision. (Show Details)
benschac edited the summary of this revision. (Show Details)
This revision is now accepted and ready to land.Apr 15 2022, 2:46 AM
This revision now requires review to proceed.Apr 15 2022, 2:07 PM
atul requested changes to this revision.EditedApr 18 2022, 10:53 AM

Do we know why this is happening? I would expect that setting the color prop to transparent would make the arrow "invisible."

I'm guessing the underlying SVG has some color value hard-coded and is "visible" because the stroke or fill or whatever is set to the correct color.

I think the right approach here might be looking at selection.json and seeing if there are any hard-coded values for the right-angle-arrow SVG.


Feel free to re-request review if I'm off-base here or missing something

This revision now requires changes to proceed.Apr 18 2022, 10:53 AM
In D3730#103649, @atul wrote:

Do we know why this is happening? I would expect that setting the color prop to transparent would make the arrow "invisible."

I'm guessing the underlying SVG has some color value hard-coded and is "visible" because the stroke or fill or whatever is set to the correct color.

I think the right approach here might be looking at selection.json and seeing if there are any hard-coded values for the right-angle-arrow SVG.


Feel free to re-request review if I'm off-base here or missing something

https://linear.app/comm/issue/ENG-1005/arrows-broken-in-safari -- yeah the commit which caused the problem is in the link of the issue.

Yeah, the color is set to currentColor which is changed in that commit. I'm just setting the color which was transparent before to transparent.

https://linear.app/comm/issue/ENG-1005/arrows-broken-in-safari -- yeah the commit which caused the problem is in the link of the issue.

I searched through the commit for color and SWMansionIcon and currentColor and transparent and couldn't find any relevant changes? Could you point out file/line number, I'm probably missing something

Looks like clearing attrs (selection.js:939) for the right-angle-arrow icon fixes the issue:

Before:

2624.png (448×964 px, 28 KB)

After:

6b3c.png (332×572 px, 16 KB)

lets tuple to avoid churn. I don't think that's the right solution.

@benschac has been struggling to figure out a solution to the SVG arrow for quite some time... at this point, would really appreciate if @atul could step in and make sure Ben is able to unblock himself ASAP. Whatever is necessary (eg. a Tuple session), let's get that done ASAP so @benschac can land a solution to this issue. (Ideally in the future @benschac can handle this stuff fully independently)

To be clear, this isn't an endorsement of any of @atul's proposals/analysis above – just a reflection that this need to be unblocked ASAP, and it looks like @atul's direct involvement is necessary at a deeper level to make that happen

tomek requested changes to this revision.Apr 20 2022, 9:42 AM

@benschac @atul have you had a chance to discuss a solution here?

This revision now requires changes to proceed.Apr 20 2022, 9:42 AM