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

πŸ’„ Reusable ASA search table #987

Conversation

treasurechic
Copy link
Contributor

β„Ή Overview

closes #985

πŸ“ Related Issues

πŸ” Acceptance:

  • yarn test passes

@vercel
Copy link

vercel bot commented Feb 28, 2023

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated
algodex-react-dev βœ… Ready (Inspect) Visit Preview πŸ’¬ Add your feedback Mar 3, 2023 at 4:20PM (UTC)

Copy link
Collaborator

@ericsharma ericsharma left a comment

Choose a reason for hiding this comment

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

Can you add a fourth column to the beginning of const columns for the asset id?

Also can you change the behavior so it works like the search table on the trade page? We want this to be a drop down table so it only is displayed when a user clicks on the asset Id search field.

@stephclarkga Do you have any feedback regarding the style of the table?

@stephclarkga
Copy link

Yeah - I was thinking it could look like this when a user clicks on the search field. I don't think we need price info here, just the token symbol, name and ASA ID. Would be cool if we can have them be sortable alphanumerically in this chart with the arrows in the header like on the Dex. And also as the user types info, this should also filter the list down

Screen Shot 2023-02-28 at 1 06 35 PM

Copy link
Collaborator

@ericsharma ericsharma left a comment

Choose a reason for hiding this comment

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

The search table should appear when a user clicks on the form field. Right now it appears when a user types into the form field.
The search table should disappear when a user clicks an element in the search table or when the form field is no longer in focus.

I would recommend switching out the onChange for an onFocus event. See the documentation for more details

@treasurechic
Copy link
Contributor Author

The search table should appear when a user clicks on the form field. Right now it appears when a user types into the form field. The search table should disappear when a user clicks an element in the search table or when the form field is no longer in focus.

I would recommend switching out the onChange for an onFocus event. See the documentation for more details

Yea, I figured. About to push the update

@ericsharma
Copy link
Collaborator

@stephclarkga Are we going to have a dropdown search table for the create token sale and mange token sale components as well? That was the way I was envisioning it.
And if so, should we vary the columns of the search table depending on the page or just have the Symbol, Name, and Asset Id columns you suggested across the board?
I think that the Quantity column is something that should be included on the search table for the create/manage token pages.

@stephclarkga
Copy link

@ericsharma Yes - should have the search table for those pages as well. I agree that the quantity in the wallet is useful for the search table for those pages. I don't think quantity is necessary on the manage token page and just clutters the table there. Added that column in the Figma. I'd make the header "Available Balance" and it should only be the amount available to trade.
Screen Shot 2023-03-02 at 11 49 24 AM

@treasurechic
Copy link
Contributor Author

@stephclarkga , @ericsharma the update is live, you guys can check now

Copy link

@stephclarkga stephclarkga left a comment

Choose a reason for hiding this comment

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

Couple formatting things:

Can you make the chart headers a bit narrower (probably cut the height by 2/3rds)?
And can the sorting arrows match the type we have in the Dex currently like I have on the Figma? I think it looks better to have the same style and have them always be visible

@stephclarkga
Copy link

Now it's showing words for ascending/descending? I just wanted the arrows to be visible so it's clear that we have sorting options for each column. Screen Shot 2023-03-02 at 3 41 53 PM

I just wanted the sort arrows to match the style we use on the search chart in the Dex like this:

Screen Shot 2023-03-02 at 3 47 33 PM

Is that possible?

As stated above, I also think the chart would look bigger if the headers were a bit narrower in height like in my screenshot above but that's a minor thing

@treasurechic
Copy link
Contributor Author

treasurechic commented Mar 2, 2023

@stephclarkga, I understand your point, I am not done with the update yet. Still working on it, will let you know when I push the update

Copy link

@stephclarkga stephclarkga left a comment

Choose a reason for hiding this comment

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

Format of the chart is good - only thing is we can only sort by one column at a time so the columns that are not actively sorting should have two gray arrows instead of having the top one revert back to green

Copy link

@stephclarkga stephclarkga left a comment

Choose a reason for hiding this comment

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

Looks good now! πŸ‘πŸ»

@ericsharma ericsharma merged commit 9c7377b into 942-feature-algodex-launchpad-for-creating-and-listing-asas Mar 3, 2023
@ericsharma ericsharma deleted the 985-modify-the-choose-asset-input-field-on-token-sale-page branch March 3, 2023 16:40
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.

3 participants