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 toolbar duplicate tools bug #3757

Closed

Conversation

engin-can
Copy link

@engin-can engin-can commented May 14, 2024

Solves issue where toolbar overflow menu has duplicate tools from main toolbar.

Before
image

After
CleanShot 2024-05-14 at 14 56 52

Change Type

  • sdk — Changes the tldraw SDK
  • dotcom — Changes the tldraw.com web app
  • docs — Changes to the documentation, examples, or templates.
  • vs code — Changes to the vscode plugin
  • internal — Does not affect user-facing stuff
  • bugfix — Bug fix
  • feature — New feature
  • improvement — Improving existing features
  • chore — Updating dependencies, other boring stuff
  • galaxy brain — Architectural changes
  • tests — Changes to any test code
  • tools — Changes to infrastructure, CI, internal scripts, debugging tools, etc.
  • dunno — I don't know

Copy link

vercel bot commented May 14, 2024

@ecan-dearx is attempting to deploy a commit to the tldraw Team on Vercel.

A member of the Team first needs to authorize it.

@huppy-bot
Copy link
Contributor

huppy-bot bot commented May 14, 2024

Hey, thanks for your pull request! Before we can merge your PR, you will need to sign our Contributor License Agreement by posting a comment that reads:

I have read and agree to the Contributor License Agreement.

@engin-can
Copy link
Author

I have read and agree to the Contributor License Agreement.

@huppy-bot huppy-bot bot added bugfix Fixes a bug of some kind sdk Affects the tldraw sdk labels May 14, 2024
@engin-can engin-can changed the title Fix bug where tools in toolbar show up in overflow menu while still v… Fix toolbar duplicate tools bug May 14, 2024
@mimecuvalo mimecuvalo mentioned this pull request May 20, 2024
13 tasks
@steveruizok
Copy link
Collaborator

Wow, how did I never notice this one? If we do remove a shape like this, I suppose that means we can restore one of the commented out tools, too.

@mimecuvalo
Copy link
Member

@steveruizok i'm closing this one - it's superseded by #3779
i'll pull over your re-ordering commit to the other PR

@mimecuvalo mimecuvalo closed this May 21, 2024
github-merge-queue bot pushed a commit that referenced this pull request May 21, 2024
Small side quest: This reworks the overflow toolbar css to rely on
`nth-child` instead of putting together a long selector.
This also address the minor issue/edge case raised in
#3757

### Change Type

<!-- ❗ Please select a 'Scope' label ❗️ -->

- [x] `sdk` — Changes the tldraw SDK
- [ ] `dotcom` — Changes the tldraw.com web app
- [ ] `docs` — Changes to the documentation, examples, or templates.
- [ ] `vs code` — Changes to the vscode plugin
- [ ] `internal` — Does not affect user-facing stuff

<!-- ❗ Please select a 'Type' label ❗️ -->

- [ ] `bugfix` — Bug fix
- [ ] `feature` — New feature
- [x] `improvement` — Improving existing features
- [ ] `chore` — Updating dependencies, other boring stuff
- [ ] `galaxy brain` — Architectural changes
- [ ] `tests` — Changes to any test code
- [ ] `tools` — Changes to infrastructure, CI, internal scripts,
debugging tools, etc.
- [ ] `dunno` — I don't know

### Release Notes

- Toolbar: cleanup overflow css rules.

---------

Co-authored-by: Steve Ruiz <steveruizok@gmail.com>
@steveruizok
Copy link
Collaborator

Correction, this isn't a bug! And if we remove the duplicated tool then actually the menu gets too shuffled. Sorry @engin-can, closing this one!

@ecan-dearx
Copy link

ecan-dearx commented May 23, 2024

So are you saying you are keeping duplicate tools in this case?
This doesn't make any sense...
CleanShot 2024-05-22 at 19 12 05

@mimecuvalo
Copy link
Member

It's only an issue if there's one item in the overflow menu (and that's been resolved in this PR: #3779)

The popout menu is meant to be stable and not shift as you select an item. (otherwise it would get confusing as items moved around)
The item that is 'duplicated' is a slot that is the most-recent item selected from that menu.

It may seem strange but it's the intended design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a bug of some kind sdk Affects the tldraw sdk
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants