fjswayze/shack

shack's Issues

DD # 2 - Backend Routes, Sample State, & Frontend Routes

Excellent work Francis! Here's the feedback on the rest of your docs:

SAMPLE STATE

• Messages: parentMessageId is a foreign key, so you'd only ever have on id and it doesn't need to point to an array
• Messages: should hold reference to when they were created so you can sort messages chronologically
• Users: should also have a property pointing to their profile picture
• Users: I'd suggest holding onto an array of the ids of the channels, at least, and maybe the dm convos that a user is a part of. Reasoning for channels: the state might at some point include channels a user is not a part of, due to browsing / searching channels. So you'd need to know which ones to render in the sidebar as the user's channels. For DM convos, can't think of a reason state would include convos that don't belong to the current user, so may not need that array for DM convos, but just something to think about.
• Channels and DirectMessages: should hold reference, via a property pointing to an array of ids, to the messages they each contain, as well as the users they contain, for easy retrieval from state (general rule of thumb: if a resource has a has_many relationship with another resource, you'll probably want to have an array of all those ids of the second resource in the frontend state)
• Memberships: no need for joins table to have their own entities slice of state. (channels and direct messages should hold reference to the users contained within them)

BACKEND ROUTES

• Nested routes should be organized by the actual resource that's retrieved, not what they're nested under (eg api/channels/:id/users would be listed under users). Not a huge deal, but helpful organizing since that corresponds with the controllers / views you'd be using (the example I provided would be handled by Users controller and which would render user views)
• GET /api/users/:id should not retrieve info that isn't stored in your db, so you can omit local time, skype, phone number, what I do;
• users: no need for PATCH or DELETE unless you include editing and deleting profiles as MVPs
• channels: you have no channels index; you'd need that for searching / browsing all channels. You might also need it for getting a user's channels; this could be distinguished from the route that retrieves / searches all channels either by checking for the presence of a query string, or by nesting the route for retrieving a user's channels under /users. Getting a user's channels could also be retrieved via the user show route (up to you); you should be able to retrieve all a user's channels somehow though, and there's no API endpoint that does that for you yet
• channels: currently you have no channel show route (GET /api/channels/:id); you should probably have one so that you can retrieve data specific to a channel; you might consider retrieving all the messages / users with the channel show, up to you, but that might make the messages and users index routes that are nested under channels moot
• direct_messages: same thing as ^, but for direct_messages; this is optional however, because as of now in your schema, direct_messages don't really hold their own data, they just have many users and many messages via joins tables; if you did have a show, it would presumably just retrieve all the users and messages for that direct_message
• add endpoints for adding or removing users from channels and direct_messages (they should be post and delete routes; let me know if you have questions about which path to use -- there's a variety of valid options for how to structure these); in this case, this would make your PATCH for direct_messages (the last route you wrote) moot
• PATCH /api/channels/:id/messages/ does not need to be nested under channels (you're editing an existing message, so you don't need to specify the channel it's a part of), but does need to have a wildcard for the message id, so you know which message to edit; same goes for the messages PATCH nested under direct_messages
• need routes for deleting a message and deleting a direct_message convo

FRONTEND ROUTES

• no need to nest your channel or dm routes under users/:userId (you'll never need to retrieve the current user's id from the path)
• since you have browsing channels as an MVP, I'd recommend a path / components for that (channel browser is a separate page / pane in the real slack)
• the NavBar and Footer, as you currently have it structured, would show on every page; however, splash has its own navbar and footer, and login / signup don't have any. So I'd nest those components under the splash route
• as a result of ^, I'd create a separate component for the primary navbar that appears when you are logged in, which appears with all the other paths (anywhere the sidebar shows up). You could clean this up by having all the main, logged in paths nested under some path like /app or /main or even just user (like you have them all nested under users/:userId now). In terms of structuring your design doc, then, you could do describe that like this:
  • /user
    • AppNavBar
    • SideBar
    • (the following routes render components here)
  • user/first_nested_path
    • Component
    • etc
• ChannelComponent & DirectMessageComponent: I'd recommend adding a sub-component for the details that show at the top of a channel / dm convo
• It looks like clicking the new message button / new direct message in the sidebar actually opens up a whole new page / pane in Slack (not a modal), so probably NewMessageForm and DirectMessagesForm shouldn't be rendered by the Sidebar, but instead should get their own path

User Auth

Nice work Francis! Love the custom slack images and icon! Below is my feedback

Functionality

• make the logos links on splash
• we already talked about this, but make sure the homepage is not an auth route (and thus in the upper right, when logged in, you should have a Launch Slack button, like in the real slack --> that can just take you to the user's default channel instead of what slack does though)
• errors from signing up appear on the login form and vice versa. either store sign up and login errors under separate keys in your state, or clear your errors somewhere (I'd advise dispatching an action to clear errors in componentWillUnmount for your session forms)
• optional / bonus for session forms: use html email validations as in the real slack https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/email#Validation

Styling

• Splash
  • add hover effect to buttons (cursor should look like pointer, color should darken)
  • make the navbar a bit taller (so there's more space above and below the buttons / logo in the navbar)
  • Sign in should look like it does on slack (just a link instead of a button)
  • Quick optional fix: I'd suggest giving a bit of space below the second splash image so the white background in the image isn't flush with the footer.
• Session Forms
  • make the button submission button a little shorter
  • add more space above the logo at the top of the page
  • render the errors above the submission button
  • change the type on your password input so that it appears as dots and not text

DD #1

Really good start to your design docs, Francis! Here's my feedback so far:

MVP LIST

• Re-order your MVPs so that hosting on heroku is first, and production README is last
• Change the dates keeping in mind that the first day of fullstacks is the 21st (your first feature says the 19th)
• User Auth: which features are only available to logged in users?
• I’d suggest re-ordering such that live chat comes after channels (the live chat would appear within channels, and also is complicated to implement)
• Channels: spell out all the CRUD functionality: creating and deleting channels (creating would make someone admin of channel), and possibly updating (adding members)
• Channels: Where do users see channels (that they are a part of)? How / where can users join channels?
• You may want to consider making pinned messages and message reactions bonus, up to you
• Direct Messages: spell out the CRUD functionality (deleting and editing)
• Direct Messages: how do users initiate a direct message? Where can a user see a list of all their direct message conversations?

SCHEMA

• All: make sure to make every column and table name snake_case (no spaces)
• Users: name profile picture with id to make clear it’s a foreign key to the profile pictures table
• Users: what does the channels column refer to? If it’s to hold reference to the channels a user is a part of, consider using a joins table instead (many to many relationship); this joins table should probably reference a user's id and the id of either a channel or a dm convo (polymorphic association)
• Users: likewise for direct messages; if you’re talking about conversations (containers for dms, ala channels), consider just using your separate direct_messages table with the joins table described above
• Messages: should belong to a channel or dm conversation; consider using a polymorphic association there (for belonging to either a channel or a dm convo)
• Messages: parent_message should allow null for top level messages
• Channels: consider adding a foreign key for admin, if you want channels to have admins (also, and this could bonus, but you may want to allow channels to be private as well)

Channels

Great work, Francis -- keep it up! Here's my feedback for the channels MVP:

Functionality

• I'd recommend removing any options from the upper right dropdown that involve features you don't plan to implement this week. (Perhaps all but log out?)
• Clicking the Channels collapse/expand button in the sidebar should show and hide all channels (the default channel always shows for me), and should default to open
• Got this when clicking on a channel in the sidebar, sometimes
• Get this upon refresh of channel show page, sometimes
• Adding members to a channel should replicate the experience on Slack: it should be a modal form with a search box. (I have some styling comments as well, but will wait for you to refactor it into modal, since the design will probably change).
• Clicking on the members number / icons in the upper right of a channel should bring up a modal like Slack's, with a header, link to add people, and a list of the members with a button in each list item to remove them; I'd say the search bar is a bonus / stretch.
• A user should automatically be a member of any channel they create (so that channel would appear in the sidebar as soon as they create it)
• When you click to join a channel in the channel browser, the join button should become a Leave button, like on Slack
• As soon as you join a channel, it should appear in the sidebar --> you might want to think about how the RECEIVE_MEMBERSHIP action might be listened to by the user / channel reducers.
• A user should not be able to join a channel they are already a part of.
• Bonus / stretch: Add a search bar to your channel browser

Styling

• The upper right user dropdown needs to look closer to Slacks --> name in bold and icon at top, more spaced out, some space between the dropdown and the right edge of the screen, drop-shadow, border radius, text turns white while its list item is hovered over, etc
• Sidebar and channel area should extend to the bottom of a page (and if there's only a few messages, those should appear at the bottom).
• I think it's fine that you made the info button in the upper right of a channel open up a dropdown instead of a whole side panel (since you won't be implementing most of those features). However, I'd style that dropdown more nicely (as with the upper right user dropdown), and would consider implementing the side panel as a bonus.
• channels in sidebar should be more spaced out and should get a hover effect like Slack's
• the little triangle in the Channels collapse / expand button should rotate depending on whether the channels index is expanded or collapsed, and should have a hover effect like Slack's
• the Channels + dropdown (browse channels, create a channel), should hide when you click outside it or click one of its options
• The members # and Add member buttons in the upper right of a channel need hover effects like Slack's
• I'd add more padding to the Create button in the Create Channel modal, as well as make sure the Create button, toggle, and input fields extend to the right side of the modal.
• Browse Channels page should have a header that says Channel browser and has a Create Channel button in the right.
• Channels in channel browser should only show join button upon hover
• Optional / bonus: center icons within their hover effect container (e.g. the plus button for creating a channel appears in the top of the light grey box behind it, upon hover)
• Optional / bonus: show up to three icons of the members of a channel in the upper right of a channel