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

feat: network selector on explorer #4863

Merged
merged 5 commits into from
Feb 2, 2023
Merged

Conversation

preschian
Copy link
Member

@preschian preschian commented Jan 30, 2023

Thank you for your contribution to the KodaDot NFT gallery.

👇 _ Let's make a quick check before the contribution.

PR Type

  • Bugfix
  • Feature
  • Refactoring

Context

Before submitting pull request, please make sure:

  • My contribution builds clean without any errors or warnings
  • I've merged recent default branch -- main and I've no conflicts
  • I've tried to respect high code quality standards
  • I've didn't break any original functionality
  • I've posted a screenshot of demonstrated change in this PR

Optional

  • I've tested it at </bsx/collection>
  • I've tested PR on mobile
  • I've written unit tests 🧪
  • I've found edge cases

Had issue bounty label?

  • Fill up your KSM address: Payout

Community participation

Screenshot 📸

  • My fix has changed something on UI; a screenshot is best to understand changes for others.

image

@netlify
Copy link

netlify bot commented Jan 30, 2023

Deploy Preview for koda-nuxt ready!

Name Link
🔨 Latest commit 96a04e9
🔍 Latest deploy log https://app.netlify.com/sites/koda-nuxt/deploys/63da7282a7ef5b0008972fa4
😎 Deploy Preview https://deploy-preview-4863--koda-nuxt.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@yangwao
Copy link
Member

yangwao commented Jan 30, 2023

powerful

@exezbcz
Copy link
Member

exezbcz commented Jan 30, 2023

Hiii, nice
one little change from the sort by issue

  • text to black - on k-accent(primary) we dont use white color because there is smaller contrast than with black.
    image

can we please make the button work as in gallery chart; when you choose sub-option it became visible on the button.
so when I chose basilisk
image

image

  • the button should have the text - basilisk

Will we enable multi-select like on sort by? (not sure in this stage) If so, the number of activated options would be useful as well. cc @yangwao
Thanks!

@preschian
Copy link
Member Author

text to black - on k-accent(primary) we dont use white color because there is smaller contrast than with black.

noted, will update the parent branch

can we please make the button work as in gallery chart; when you choose sub-option it became visible on the button.

in the current stage, the network is already selected
how about change the text to this for now: Network: Basilisk

Will we enable multi-select like on sort by? (not sure in this stage)

not in the current stage I guess. we can achieve it once we implemented fetching multi-chain

@preschian
Copy link
Member Author

can we please make the button work as in gallery chart; when you choose sub-option it became visible on the button.

in the current stage, the network is already selected how about change the text to this for now: Network: Basilisk

need your feedback on this one cc @exezbcz @yangwao

@preschian preschian marked this pull request as ready for review February 1, 2023 04:37
@preschian preschian requested a review from a team as a code owner February 1, 2023 04:37
@preschian preschian requested review from roiLeo and Jarsen136 and removed request for a team February 1, 2023 04:37
Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

in the current stage, the network is already selected how about change the text to this for now: Network: Basilisk

^ yes!

at the moment it's confusing to have written "All Networks" while only one is showing/selected

@codeclimate
Copy link

codeclimate bot commented Feb 1, 2023

Code Climate has analyzed commit 96a04e9 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

@exezbcz
Copy link
Member

exezbcz commented Feb 1, 2023

can we make it to only display the network name without the "network:"

  • now the button is changing its width based on how long the name is - can we make it fixed?
    image
    otherwise alright! thanks

@roiLeo
Copy link
Contributor

roiLeo commented Feb 1, 2023

now the button is changing its width based on how long the name is - can we make it fixed?

are we sure about fixed width? it can have an impact on responsive mode

(note: we don't have Senk, Moonriver & Moonbeam on production so no long names)

},
query: {
page: '1',
sort: route.query.sort,
Copy link
Contributor

Choose a reason for hiding this comment

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

how about adding the search keywords to query

Copy link
Member Author

Choose a reason for hiding this comment

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

will add this 👍

@exezbcz
Copy link
Member

exezbcz commented Feb 1, 2023

now the button is changing its width based on how long the name is - can we make it fixed?

are we sure about fixed width? it can have an impact on responsive mode

(note: we don't have Senk, Moonriver & Moonbeam on production so no long names)

hmm, what about something like minimal width? what are the other options how we can avoid button jumping? - there will be a layout change button in future - which would be influenced by this.
wdyt?

@yangwao
Copy link
Member

yangwao commented Feb 1, 2023

Will we enable multi-select like on sort by?

probably not in this

how about change the text to this for now: Network: Basilisk

yes, could be

Terms of stuff in button, I would keep for sake clarity end user "Network" there as prefix, yet on dropdown could be w/o

rarible
image

@yangwao
Copy link
Member

yangwao commented Feb 1, 2023

btw, sry disturbing lovely discussion, yet happy to move on and not get stuck on wording :D

@yangwao
Copy link
Member

yangwao commented Feb 2, 2023

hmm could be fixed in other issue I guess?
Why it influences width of search bar?

Screen.Recording.2023-02-02.at.10.05.33.mov

@yangwao
Copy link
Member

yangwao commented Feb 2, 2023

can we make it to only display the network name without the "network:"

we can still use some sort of label. People outside of Ethereum barely knows what are names of other parachains in Polkadot, not even canary networks on Kusama

@yangwao
Copy link
Member

yangwao commented Feb 2, 2023

what are the other options how we can avoid button jumping? - there will be a layout change button in future - which would be influenced by this.

we can reflect in explorer feedback

@yangwao
Copy link
Member

yangwao commented Feb 2, 2023

pay 50 usd

@yangwao yangwao merged commit 738f80b into main Feb 2, 2023
@yangwao yangwao deleted the feat/explorer-chain-selector branch February 2, 2023 09:08
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.

Explorer Network switch
5 participants