aburd / beats-with-friends

A piece of software where beatmakers can make beats with each other.
3 stars 0 forks source link

Feature/user list #9

Closed kimm950 closed 2 years ago

kimm950 commented 2 years ago

IMPORTANT: Please do not create a Pull Request without creating an issue first. closes https://github.com/aburd/beats-with-friends/issues/8 closes #7

*Any change needs to be documented in issues so that people are aware of what's being developed.

UI improvement, adding user list feature

Nope

Screen Shot 2022-08-19 at 16 58 28 Screen Shot 2022-08-19 at 17 12 49
aburd commented 2 years ago

@kimm950 Thanks for this PR! I'm actually working on something somewhat similar. If it's OK, would you mind if I merged #10 with this one?

kimm950 commented 2 years ago

@aburd sure!!

aburd commented 2 years ago

@kimm950 I messed up this PR bad...somehow when I rebased it got rid of your commits, but I think if you squash merge this, you should get the commit anyway.

aburd commented 2 years ago

As for the PR itself, the way you did it made me rethink how it should be approached! I like your approach of just putting the group stuff on the bottom. So I changed the code to just keep all the "group stuff" in a GroupMenu component!

If you're happy with this, please go ahead and merge!

aburd commented 2 years ago

Screenshot from 2022-08-22 20-10-53 Screenshot from 2022-08-22 20-11-14

aburd commented 2 years ago

@kimm950 Also, I wasn't really picky about the styling, so my code may have messed a bit with what you're trying to do (put the group styling in the sequencer). If you wanna change that back go for it!

I really liked what you did with the simple approach of making the active user orange so I definitely wanted to keep that.

kimm950 commented 2 years ago

I messed up this PR bad...somehow when I rebased it got rid of your commits, but I think if you squash merge this, you should get the commit anyway.

It's all good!

If you're happy with this, please go ahead and merge!

I am happy, let me merge this!