add ability to hide full or locked tables

[yurii-lubynets]

closes: https://github.com/sparkletown/internal-sparkle-issues/issues/851

[qlty-cloud-legacy]

Code Climate has analyzed commit cef296b and detected 0 issues on this pull request.

View more on Code Climate.

[github-actions]

Visit the preview URL for this PR (updated for commit cef296b):

https://co-reality-staging--preview-pr-1706-e7wrr2bl.web.app

_{(expires Tue, 20 Jul 2021 13:28:59 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

[0xdevalias]

We have tables in more locations than just the jazz bar. This should be applied to all of them. And utils/shared components extracted to support this feature in all locations without duplicating the code.

Further specific review comments to come when I’m on a computer.

[sunny-viktoryia]

We have tables in more locations than just the jazz bar. This should be applied to all of them.

@yurii-lubynets have you fixed that and is the PR ready for the review?

[yurii-lubynets]

@sunny-viktoryia
The PR is ready for review. The feature is available for Jazzbar, Conversation space templates as per comment here https://github.com/sparkletown/internal-sparkle-issues/issues/851#issuecomment-873308110

[sunny-viktoryia]

few more comments from me. The logic seems correct now. The majority of the comments are about the variable names.

Although I have raised concern here about the performance. Not sure if it's a good idea to iterate through the online users several times (looks like we do that for all the tables twice, would be great if we could that only once if possible).

I believe we could create a key-value object which would store the { [table.reference]: usersAtTableArray } and then could use that information where needed:

• if the table is empty
• if the table is full (twice)

so would be great if @yurii-lubynets could do the refactoring of that part (if you have the capacity for that). Not sure if it has a huge performance impact though.

[dreaming] definitely OOS, in an ideal Sparkle ✨ would be cool to be able to run emulators for each venue type to test the performance and see how our changes impact that. mmmmmm.

[yurii-lubynets]

few more comments from me. The logic seems correct now. The majority of the comments are about the variable names.

Although I have raised concern here about the performance. Not sure if it's a good idea to iterate through the online users several times (looks like we do that for all the tables twice, would be great if we could that only once if possible).

I believe we could create a key-value object which would store the { [table.reference]: usersAtTableArray } and then could use that information where needed:

• if the table is empty
• if the table is full (twice)

so would be great if @yurii-lubynets could do the refactoring of that part (if you have the capacity for that). Not sure if it has a huge performance impact though.

[dreaming] definitely OOS, in an ideal Sparkle ✨ would be cool to be able to run emulators for each venue type to test the performance and see how our changes impact that. mmmmmm.

@sunny-viktoryia
have refactored <TablesUserList/> to store the { [table.reference]: usersAtTableArray } and use this information where needed. Please review

[sunny-viktoryia]

suuuuuper clooooooooose!! 🙃
just a couple of variable name changes and should be good to go,

[sunny-viktoryia]

mmmm, finally, approving!! Looks good to merge. 🙃 🎉

Thanks for your patience and for being open to fix my requested changes. We did it! It looks so much better to my eye than the 1st version! 😍

looks like I've fixed all requested changes