Slider upgrade

[xarvic]

Adding RangeSlider and Annotated.
Adding options track_color, axis, knob_style and annotated on Slider and RangeSlider.

[jneem]

Is there something I can do here to help? I'm not particularly familiar with the slider, but if @PoignardAzur is otherwise busy, I can take a look.

[PoignardAzur]

Sorry, I looked at the code, meant to write a review, but I forgot about it.

Overall, I think the code is pretty good. It's clean, it's mostly straightforward, no major flaw stands out.

Some additional notes:

• The druid codebase tries to avoid merge commits. You should probably rebase your PR onto master, and batch the changes into fewer commits while you're at it.
• You need to add a short description your example to druid/examples/readme.md.

Overall, I think this code is ready to merge. The big blockers are the problem of low step values with Annotated and the examples/ readme. All the other changes I suggest are nice-to-haves.

[PoignardAzur]

@jneem I'm not too occupied, I just procrastinated on it. But thanks for the offer. =)

[xarvic]

Thanks for your review and sorry that it took so long, i was quite busy lately.

I addressed all of your suggested changes except the merge commits. Can i get rid of them by squash-merging the branch? These commits were created by Github, after resolving the merge conflicts is there another way to resolve merge commits without creating merge-commits?

The other thing i dont understand is this error:

   Compiling druid-shell v0.7.0 (/Users/runner/work/druid/druid/druid-shell)
error: use of deprecated macro `test`: test-env-log has been renamed to test-log; use it instead
Error:    --> druid-shell/src/backend/mac/menu.rs:185:7
    |
185 |     #[test]
    |       ^^^^
    |
    = note: `-D deprecated` implied by `-D warnings`

error: use of deprecated macro `test`: test-env-log has been renamed to test-log; use it instead
Error:   --> druid-shell/src/backend/mac/screen.rs:87:7
   |
87 |     #[test]
   |       ^^^^

error: use of deprecated macro `test`: test-env-log has been renamed to test-log; use it instead
Error:   --> druid-shell/src/backend/mac/screen.rs:94:7
   |
94 |     #[test]
   |       ^^^^

error: use of deprecated macro `test`: test-env-log has been renamed to test-log; use it instead
Error:    --> druid-shell/src/backend/mac/screen.rs:113:7
    |
113 |     #[test]
    |       ^^^^

error: could not compile `druid-shell` due to 4 previous errors
Error: warning: build failed, waiting for other jobs to finish...
error: build failed
Error: The process '/Users/runner/.cargo/bin/cargo' failed with exit code 101

I didn't changed anything in druid_shell. Has is something to do with merging?

I am not a Git expert any advice with these two things is appreciated :)

[CryZe]

Feel free to cherry-pick this commit that fixes the CI issue: db7faff

[jneem]

What's the status of this? I'm happy to do the squash/rebase if it's all that's needed.

[jneem]

@PoignardAzur ?

[PoignardAzur]

Oh, sorry.

I'm currently busy catching up on other work, so if you want to take responsibility for this PR, feel free =)

Just ping me once you need it merged.

[xarvic]

What's the status of this? I'm happy to do the squash/rebase if it's all that's needed.

Yes, thank you. The problem is i used merge commits to update my fork and i don't know how to remove them from this PR and i need to cherry-pick db7faff but i don't know how that works.

[jneem]

No problem, I can probably handle it today. Any objections if I just modify this PR directly?

[xarvic]

No, that's fine, thanks :)

I think these changes have been made already

[jneem]

Ok, I think the git/CI issues are resolved now.

[jneem]

For future reference, it's best not to merge changes from the main branch into a PR: it creates duplicate commits, because when git merges it creates new commits that "look like" the original commits but have different ids. And then when you try to merge back there are multiple commits all trying to do the same thing.

The fix was:

$ git rebase -i 8fc3e5f105d0
(then select only the commits that were relevant to this PR. This creates a clean history, but based on the (old) commit 8fc3e5f105d0)
$ git rebase master
(this rebases the clean history so that it's on the current, up-to-date master)

(Edit: sorry, I just realized from scrolling up that you already know about the issue with merge commits. Didn't mean to pile on 😄)

[xarvic]

Thanks a lot. I am going to try to understand what you did there :)

[PoignardAzur]

git rebase -i is very user-friendly. It pops up a file with a list of commands that explains to you what it does =)