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

feat(brush): disable drag overlay #1744

Merged
merged 8 commits into from
Dec 7, 2023

Conversation

Onxi95
Copy link
Contributor

@Onxi95 Onxi95 commented Aug 25, 2023

Hi! Thank you for this wonderful library - without you I would probably cry in the corner thinking how to implement fancy charts 😅

🚀 Enhancements

  • added disableDraggingOverlay to Brush in order to be able to use Brush only as a scroll (without possibility of changing its size)

🤔 Motivation

I was building something similar to bar chart. The requirements changed a bit (ofc 😅) and the chart has to have multiple columns with minimum width and scrollbar if needed. The closest solution to achieve this result is Brush (It doesn't handle scroll events, but manual scroll using mouse is good enough for our use-case).
I styled the brush to look like a standard, gray scrollbar, but when I click outside of it, new area of brush is created. There's disableDraggingSelection prop, but it affects scrollbar, and there's no option to do something similar but to overlay. I managed to work around it by using pointer-events: none, but I think it's a bit hacky solution and making it available in the lib might be beneficial :D

🧪 How to test it?

in brush example (packages/visx-demo/src/sandboxes/visx-brush/Example.tsx), adjust <Brush /> component like:

          <Brush
            xScale={brushDateScale}
            yScale={brushStockScale}
            disableDraggingOverlay // <- new prop
            width={xBrushMax}
            height={yBrushMax}
            margin={brushMargin}
            // handleSize={8}
            innerRef={brushRef}
            // resizeTriggerAreas={['left', 'right']}
            brushDirection="horizontal"
            initialBrushPosition={initialBrushPosition}
            onChange={onBrushChange}
            // onClick={() => setFilteredStock(stock)}
            selectedBoxStyle={selectedBrushStyle}
            useWindowMoveEvents
            // renderBrushHandle={(props) => <BrushHandle {...props} />}
          />
brush.mp4

@Onxi95
Copy link
Contributor Author

Onxi95 commented Nov 19, 2023

Hi @williaster 👋 Can you take a look at it when you have some time? 😄

@williaster
Copy link
Collaborator

Hi @Onxi95 👋 thanks for the ping, sorry this slipped through. It looks like CI hasn't run for the PR, can you try to rebase just to re-trigger things? I'll try to take a look at it ASAP, it may be next week due to the holiday this week

@Onxi95
Copy link
Contributor Author

Onxi95 commented Nov 21, 2023

Hello @williaster 👋

I was looking around, trying to see what's going on with actions, but at the end I saw that

1 workflow awaiting approval
image

I guess it's needed to trigger actions and I think I don't have permissions to do it 😅

Launched the jobs from pull_request.yml (skipped yarn run happo-ci-github-actions & had some issues with yarn check:sizes as it relies on getGithubClient function that uses some CI envs) locally and I didn't see any issues - if I miss something, please let me know 🙏

Copy link
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

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

Thanks for your patience here @Onxi95 🙏 this lgtm, thank you for the contribution!

@williaster williaster merged commit bf2a2f3 into airbnb:master Dec 7, 2023
2 checks passed
Copy link

github-actions bot commented Dec 7, 2023

🎉 This PR is included in version v3.6.0 of the packages modified 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants