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: Fix DataDoc cell move down button #1410

Merged

Conversation

baumandm
Copy link
Contributor

DataDoc cells have ⏫ ⏬ buttons to move them up or down in the DataDoc, but it's impossible to click on the ⏬ button because the next cell's ⏫ button always gets in the way.

Each cell has a header and a footer row of buttons, and the footer row overlaps the header row of the next cell. They are set to opacity: 0 unless the cell is being hovered over, so you never see both at the same time. However, the issue is that elements on the page are automatically stacked bottom up, so that as you move down the page, each element stacks on top of earlier elements. Hovering doesn't care about order, so both the footer and the next header are both receiving the :hover style at the same time, and the stacking order puts the header on top of the footer, so you can't click the ⏬ button.

I fixed this issue by switching to display: nonefor just the header rows, which prevents them from receiving hover events. Then I moved the :hover to the .data-doc-cell-container-pair itself, instead of the button row. This means if you hover over a cell, the header/footers appear, and you can mouse over either without losing focus to nearby cells. Once you move past the headers and into another cell, the header/footer flips to the newly-hovered cell.

I left the footer rows (and the first header) using opacity: 0 to prevent layout shift: if both used display: none, the cells would move around as you hover. This could be solved by adding additional margin to the cells and negative margin to the header/footers, but this approach was easier.

Here is a video of the change in action:

Screen.Recording.2024-02-14.at.2.40.52.PM.mov

Finally⸺the down logic in moveCellAt() was broken so I implemented the easiest fix, adjusting the index calculations. The actual issue is that the index is "wrong" for the footer buttons. You can see that in this screenshot, where I show the value of the index field next to the buttons:

Screenshot 2024-02-14 at 2 29 40 PM

I assume that is intentional, and changing the index would require changes in all the other cell operations, so this seemed like the best fix.

@jczhong84
Copy link
Collaborator

thanks for the fix!

@jczhong84 jczhong84 merged commit dd185bf into pinterest:master Feb 25, 2024
3 checks passed
@baumandm baumandm deleted the external/data-doc-cell-move-down branch February 28, 2024 14:46
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.

None yet

2 participants