-
-
Notifications
You must be signed in to change notification settings - Fork 836
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#2PlaysAMonth]: Image Gallery - Create a responsive image gallery by using the free Unsplash API #974
[#2PlaysAMonth]: Image Gallery - Create a responsive image gallery by using the free Unsplash API #974
Conversation
… infinite scrolling functionality
This project contains the following principles of React 1. useState 2. useEffect 3. promise handling using async await 4. react form Following things have been used while making this project 1. API handling from unsplash 2. Grid 3. Individual components (SearchBar can be reused separately) 4. Have implemented the infinite scrolling where the images array are populated once we scroll to the bottom of the page 5. Have used the Material UI icon for search bar The project is responsive as well. Following things are pending and trying to figure out the reason for the same 1. Infinite scrolling is working on local system but when same code is being used in React play it is not working 2. Will try to add React-Router and Redux functionaly for state management.
@siddhantsiddh15 is attempting to deploy a commit to a Personal Account owned by @reactplay on Vercel. @reactplay first needs to authorize it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello! Thank you for your contribution 😊.
Please reference the issue number in the description if you are fixing a bug.
If you are implementing a feature request, please check with the maintainers that the feature will be accepted first.
Stale Marking : After 30 days of inactivity this PR will be marked as stale PR and it will be closed and locked in 7 days if no further activity occurs.
@siddhantsiddh15 , Thanks for the PR. I would request you change the PR title to "[#2PlaysAMonth]: Image Gallery - Create a responsive image gallery by using the free Unsplash API" and the second thing is to link your PR with the issue by adding the below line in the PR description. Example: Fixes #910 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@siddhantsiddh15 , Please format and lint the code by following this guide. |
Greetings Have formatted the code as per the guidelines as mentioned here https://github.com/reactplay/react-play#format-the-code and then pushed the code in the branch. Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@siddhantsiddh15 Review comments
also please record your demo on stackstream and add the link by editing your play. Thanks
@@ -0,0 +1,113 @@ | |||
/* enter stlyes here */ | |||
* { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not override the styles this way. Add a unqie class name in front of all the styles in all the CSS files so that the override changes are less.
You can add an additional style name here and use that to prefix all styles in all CSS files.
// src/plays/image-gallery-using-unsplash-api/ImageGalleryUsingUnsplashApi.js
<div className="play-details-body">
@@ -0,0 +1,112 @@ | |||
/* enter stlyes here */ | |||
.body { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have made changes here. Earlier it was *{ padding: 0;
margin: 0;
font-family: 'IBM Plex Sans', sans-serif;
background-color: whitesmoke;
} .
Now it is under a className body.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class name should prefix all the styles in the CSS file.
@siddhantsiddh15 Kindly resolve the merge conflict |
<PlayHeader play={props} /> | ||
<div className="play-details-body"> | ||
{/* Your Code Starts Here */} | ||
<div className="body"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would certainly change the class name from body to something more unique :) How about unsplash-ig
?
@@ -0,0 +1,112 @@ | |||
/* enter stlyes here */ | |||
.body { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The class name should prefix all the styles in the CSS file.
…dhantsiddh15/react-play into unsplash-api-siddhantsiddh15 Changed the className from .body to .unsplash-image-gallery-body
@siddhantsiddh15 Catch me up on Discord today to close it. |
Hello Tapas, Thankyou |
@siddhantsiddh15 almost there.. please add a cover image Also edit your play from localhost and add the stream recording. Ping where when you done, will merge it. |
@siddhantsiddh15 let us know when done |
…dhantsiddh15/react-play into unsplash-api-siddhantsiddh15 Added the cover image
Greetings, I have not added the demo recording in the Play. I have updated the size of the cover.png to 135 kb. Regards |
Can you please add the demo recording too.. then its al done. |
Cannot do it. Having difficulty. Can we skip the recording portion? |
Ok, no worries, no pressure. It's still valuable to get your work in. I am just curious about what kind of issues you are facing. If you can post about it in our Discord, I may try the resolution.. In fact, you can add the recording after merge too...(before 5th March) |
I am having slow internet connection due to the place I have travelled to, recording is a big file to upload. |
Hey @siddhantsiddh15 , this play looks cool. I will be waiting for the video link to updated before merging it to production branch |
Okay, will update it by tonight. |
I have uploaded here the updated video of the play. I have checked the responsiveness and working of my play. The delay from my side was unwanted, I have uploaded the video as soon as I got good internet connection. |
Well the video should be on https://stack-stream.com/ I can get this play merged if everything is ok however please record a stackstream video before EOD |
@atapas you need to unblock in order to merge this PR |
Description
The project contains use of Unsplash API, which is fetched using axios and then populated in the Photo Component.
Following things have been used in this project:
The app is responsive and redirects to the individual images and profiles when clicked on it.
Fixes #910
Type of change
How Has This Been Tested?
To see working of the app repeat the following
Checklist:
Screenshots or example output