srtucker22 / glipchat

video chatroom using meteor + webrtc + react + redux
https://glipchat.herokuapp.com/
MIT License
279 stars 78 forks source link

Upstreaming some potential adjustments to quasar based on a small personal co-learning exercise #6

Closed shanecoughlan closed 8 years ago

shanecoughlan commented 8 years ago

Hi Simon

Thanks for accepting my earlier push of code to your project.

Paul Cohen (sonicviz) and myself were exploring quasar as a learning tool around Meteor, React and WebRTC. We appreciated your code layout and concise design.

While playing with the code we made a few changes and we wanted to share them back to you for review and possible inclusion. These changes are split into two parts: developer onboarding and user experience.

Regarding developer onboarding we made three changes: (1) Readme improved with simpler language (already pushed to you). (2) Overview of technical architecture (tools used rather than project layout) used in the system architecture (added to expanded readme). (3) Commenting improved (licenses across files, one large file received improved example comments in JSDoc compatible format)

Regarding user experience we made two changes: (1) Control button placement improved for possible use on tablets and for preventing the buttons to overlap with eye-lines. (2) Button usability improved for discoverability. (3) Error dialogues have improved readability for non-native speakers. The "wrong browser" message also now provides links to Chrome and Firefox.

While you may or may wish to include all our changes we hope you find them interesting. We enjoyed playing with your project and - time permitting - I will personally potentially explore more details of how it works.

Regards

Shane Coughlan in partnership with Paul Cohen

PS: Part of this pull request contains historical items which were a mistake on my part when I experimented with WebSphere. It is not relevant to the submission of our project adjustment code. These historical items were submitted at: 8875a39 Run through: 7f81c5f, a156ac4, b925bf9 and 84809a6 And were reverted at: 8c0a398

sonicviz commented 8 years ago

Hi Simon, Ditto to what Shane said. It was a good learning exercise to prepare for some deeper hacking of quasar.

Hope you find the tweaks useful!

Cheers, Paul

srtucker22 commented 8 years ago

Hey guys,

Thanks so much for all your work! All the changes were excellent.

My only change was as follows: I saw you moved the video controls and had them always visible for compatibility with phones and tablets -- where hovering doesn't exist. Instead of this, took the Facetime approach, where the user can tap the screen to see the controls and tap again to get rid of them. Same goes for the small video window controls.

Looking forward to seeing a sweet app using some of this stuff in production. Let me know what you guys are planning or if you need any help!

Cheers, Simon

sonicviz commented 8 years ago

No worries Simon. UX is an interesting area, multiple ways to skin a cat;-)

I'm watching the evolution of Mantra and the debate over directory structure at the moment. https://github.com/kadirahq/mantra/issues/3

What's your preference?

srtucker22 commented 8 years ago

So I've tried both ways and my preference is type-base structured. I've heard many args for feature-based and I respect the args, but I've found that I end up thinking a lot more about where to place things when features are murky (e.g. a component that is used by multiple components and talks with multiple stores) and for consumers of the code, it can be non-obvious where those features and files are located.

With type-based, you know where everything is and it's quick to navigate. Checking between stores or components is easier for me to navigate as well. People argue that in very large applications, it might make more sense to go feature-based, but in that case I would want to refactor and turn a big feature into it's own separate package. If you do that, refactoring is same speed with both structures, and you can keep the type-based structure in the new package as well.

Honestly, one way or the other isn't a big deal to me except for the speed with which I can build/modify the app and navigate the directories, and for me that's type-based.

shanecoughlan commented 8 years ago

Hi Simon!

Thanks for merging some of the suggestions. Following the Facetime approach for the controls sounds sensible because it will match a lot of user expectations from previous experience.

At the moment I am exploring the advantages/disadvantages of Meteor for WebRTC-based communication in mobile or web apps. quasar is an attractive place to start due to the use of Meteor + React. I appreciate that you created and shared this application as Open Source.

Regards

Shane