Skip to content

Instagram challenge - #902

Open
Riky5 wants to merge 11 commits into
makersacademy:mainfrom
Riky5:main
Open

Instagram challenge#902
Riky5 wants to merge 11 commits into
makersacademy:mainfrom
Riky5:main

Conversation

@Riky5

@Riky5 Riky5 commented Jan 24, 2022

Copy link
Copy Markdown

No description provided.

@post = Post.new
end

def create

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment I can write a caption without uploading an image. This is then shown on the feed. When I upload an image without a caption it goes to the feed but it is not posted. I think some alerts would be beneficial, telling the user a post has been created or not giving reasons.

@@ -0,0 +1,27 @@
class PostsController < ApplicationController
def index
@posts = Post.all.order('created_at DESC')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clever use of ordering here

@@ -0,0 +1,20 @@
class SessionsController < ApplicationController

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good separation of classes.

end

def destroy
session[:user_id] = nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good way to log a user out, clean and simple

@@ -0,0 +1,82 @@
class Users::RegistrationsController < Devise::RegistrationsController

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When signing up I intentionally tried to put password different to confirm password. It stopped me from signing up but didnt say way, might be good to tell users why the sign up is failing. I noticed it was also failing for having a username and email set to "1". Is there a check for uniqueness and min lengths here or format?

@chris-clement chris-clement left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice work! Very high standard of CSS used, I like the sign up and login page. I would add more alerts and notices to tell the user why they are unable to signup and login e.g. email format is invalid, username too short, password and confirm password dont match. Same for creating a post. I think this is a strong start given this is the first weekend working on this. The nav bar looks good and would benefit from being expanded. Few comments below but looks very good and works well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants