Skip to content
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

Auctions page: create new auction, make bid on existing auctions #165

Merged
merged 14 commits into from
Mar 13, 2021

Conversation

swarajpure
Copy link
Contributor

@swarajpure swarajpure commented Mar 10, 2021

Fixes Real-Dev-Squad/todo-action-items#16

Unable to add GIF preview here due to size limitation.
Please check preview here: https://cdn.discordapp.com/attachments/748267291405320204/820029342876827658/auctions_gif.gif

Subsequent PRs to improve the feature/fix bugs:

@swarajpure swarajpure added the feature task A big ticket item that needs to come up as a feature label Mar 10, 2021
@swarajpure swarajpure self-assigned this Mar 10, 2021
@ankushdharkar ankushdharkar temporarily deployed to crypto-pipel-feat-aucti-nqk3wp March 10, 2021 21:29 Inactive
components/Auctions/Auctions.module.css Outdated Show resolved Hide resolved
components/Auctions/createNewAuction.js Outdated Show resolved Hide resolved
components/Auctions/createNewAuction.js Outdated Show resolved Hide resolved
components/Auctions/createNewAuction.js Outdated Show resolved Hide resolved
components/Auctions/createNewAuction.js Outdated Show resolved Hide resolved
components/Auctions/index.js Outdated Show resolved Hide resolved
components/Auctions/index.js Outdated Show resolved Hide resolved
@SBagaria2710
Copy link
Contributor

LGTM 👍🏼

@ankushdharkar ankushdharkar temporarily deployed to crypto-pipel-feat-aucti-nqk3wp March 11, 2021 19:23 Inactive
@ankushdharkar ankushdharkar temporarily deployed to crypto-pipel-feat-aucti-nqk3wp March 11, 2021 19:30 Inactive
Copy link
Contributor Author

@swarajpure swarajpure left a comment

Choose a reason for hiding this comment

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

@sumitd94 Done with the changes. Please check

sumitd94
sumitd94 previously approved these changes Mar 11, 2021
Copy link
Contributor

@sumitd94 sumitd94 left a comment

Choose a reason for hiding this comment

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

Great job 🔥

@ankushdharkar ankushdharkar temporarily deployed to crypto-pipel-feat-aucti-nqk3wp March 12, 2021 08:11 Inactive
@ankushdharkar ankushdharkar temporarily deployed to crypto-pipel-feat-aucti-nqk3wp March 12, 2021 10:39 Inactive
@ice-rahul ice-rahul self-assigned this Mar 12, 2021
@ankushdharkar ankushdharkar temporarily deployed to crypto-pipel-feat-aucti-nqk3wp March 12, 2021 19:52 Inactive
@ankushdharkar ankushdharkar temporarily deployed to crypto-pipel-feat-aucti-nqk3wp March 12, 2021 19:59 Inactive
@ankushdharkar ankushdharkar temporarily deployed to crypto-pipel-feat-aucti-nqk3wp March 12, 2021 20:39 Inactive
Comment on lines +92 to +109
.biddersImg[data-columns="2"]{
width: calc(160px/2);
}
.biddersImg[data-columns="3"]{
width: calc(160px/3);
}
.biddersImg[data-columns="4"]{
width: calc(160px/4);
}
.biddersImg[data-columns="5"]{
width: calc(160px/5);
}
.biddersImg[data-columns="6"]{
width: calc(160px/6);
}
.biddersImg[data-columns="7"]{
width: calc(160px/7);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this doing?

Copy link
Contributor

Choose a reason for hiding this comment

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

Here we are changing the size of the bidder's image
Size of the bidder's Image-based on the number of columns allowed in the container horizontally.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use a declarative class or an inline sizing?


const handleNewBid = async (e, auctionId) => {
if (!isUserLoggedIn) {
return alert('Please log in to bid!');
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT:

alert();
return;

};

const getColumns = (totalBidder) => {
return totalBidder <= 12 ? Math.ceil(totalBidder / 2) : 7;
Copy link
Contributor

Choose a reason for hiding this comment

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

What are 12 and 7?

Copy link
Contributor

Choose a reason for hiding this comment

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

MAX_ALLOWED_COLUMNS=7
MAX_BIDDERS_ALLOWED_BELOW_MAX_COLUMNS=12

@ankushdharkar ankushdharkar temporarily deployed to crypto-pipel-feat-aucti-nqk3wp March 13, 2021 19:42 Inactive
@ankushdharkar ankushdharkar temporarily deployed to crypto-pipel-feat-aucti-nqk3wp March 13, 2021 19:44 Inactive
@ankushdharkar
Copy link
Contributor

If all comments are resolved, time to ship this! 🚢

@ankushdharkar ankushdharkar merged commit 17948b6 into develop Mar 13, 2021
@ankushdharkar ankushdharkar deleted the feat/auctions-page branch March 13, 2021 19:56
@swarajpure swarajpure changed the title Auctions Page Auctions page: create new auction, make bid on existing auctions Apr 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature task A big ticket item that needs to come up as a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auction Page
6 participants