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

refactor: neo dropdown #6196

Merged
merged 18 commits into from
Jun 28, 2023
Merged

refactor: neo dropdown #6196

merged 18 commits into from
Jun 28, 2023

Conversation

preschian
Copy link
Member

@preschian preschian commented Jun 9, 2023

Thank you for your contribution to the KodaDot - One Stop Shop for Polkadot NFTs.

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

PR Type

  • Bugfix
  • Feature
  • Refactoring

Needs QA check

  • @kodadot/qa-guild please review

Context

Did your issue had any of the "$" label on it?

Screenshot 📸

  • My fix has changed UI

Copilot Summary

🤖 Generated by Copilot at 934af36

Refactored various components to use custom NeoDropdown and NeoDropdownItem components instead of Buefy components for better UI consistency, accessibility, and performance. Added a new feature to list an NFT directly from the gallery item by passing the nftId and price as parameters. Updated the NeoDropdown and NeoDropdownItem components to extend the oruga library components and allow more customization. Fixed some minor bugs and improved the styling of the dropdown components.

🤖 Generated by Copilot at 934af36

We're breaking free from the chains of Buefy
We're building our own NeoDropdown army
We're refactoring the code with fire and fury
We're improving the design, performance, and accessibility

@reviewpad
Copy link
Contributor

reviewpad bot commented Jun 9, 2023

AI-Generated Summary: This pull request includes multiple changes across various components, primarily focusing on replacing b-dropdown and b-dropdown-item with the new custom components NeoDropdown and NeoDropdownItem. The import statements have been updated to import these new components directly from the @kodadot1/brick package. The file diffs also involve refactoring and simplification of component structures, improved usage of templates and slots, and better handling of events and attributes. Several files also have minor code style improvements and enhanced readability. Additionally, the composables/transaction/types.ts file shows updates to the ActionList type, making it more flexible and customizable. Overall, this pull request improves the consistency, maintainability, and user experience of the components involved.

@reviewpad reviewpad bot added the large Pull request is large label Jun 9, 2023
@reviewpad
Copy link
Contributor

reviewpad bot commented Jun 9, 2023

Reviewpad Report

⚠️ Warnings

  • Please link an issue to the pull request

@roiLeo roiLeo mentioned this pull request Jun 9, 2023
37 tasks
Base automatically changed from feat/release-redesign-sidebar-menu to main June 15, 2023 13:55
@netlify
Copy link

netlify bot commented Jun 16, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 71485d3
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/649c03c9ab7663000884d807
😎 Deploy Preview https://deploy-preview-6196--koda-canary.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 configuration.

@preschian
Copy link
Member Author

preschian commented Jun 16, 2023

todos:

  • migrate .dropdown-menu and .dropdown-item selector

@preschian preschian marked this pull request as ready for review June 20, 2023 09:37
@preschian preschian requested a review from a team as a code owner June 20, 2023 09:37
@preschian preschian requested review from roiLeo and vikiival and removed request for a team June 20, 2023 09:37
@preschian preschian requested review from daiagi and prury June 20, 2023 09:38
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.

hover state in navbar

Enregistrement.de.l.ecran.2023-06-20.a.11.42.21.AM.mov

@prury
Copy link
Member

prury commented Jun 20, 2023

  • Create on Statemine:
    image

  • Chart dropdown broken on mobile
    Dropdown options won't show

  • Mobile issues(on both items and gallery pages):
    image
    image
    image
    image

@prury prury added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Jun 20, 2023
@preschian
Copy link
Member Author

@roiLeo @prury updated

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.

✅ wfm

@roiLeo roiLeo added the S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked label Jun 21, 2023
@vikiival
Copy link
Member

Screenshot 2023-06-21 at 14 25 05

@prury
Copy link
Member

prury commented Jun 21, 2023

Another one(unlockables mobile):
image

@roiLeo roiLeo added S-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved and removed S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked labels Jun 21, 2023
@preschian
Copy link
Member Author

Another one(unlockables mobile):

updated

@prury prury added S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked and removed S-changes-requested-🤞 PR is almost good to go, just some fine tunning labels Jun 22, 2023
@daiagi
Copy link
Contributor

daiagi commented Jun 23, 2023

is there something that can be done about the repeating
<template #trigger="{ active }"> ?

  1. can we make trigger active by default
  2. what are the other options except for active?

@roiLeo
Copy link
Contributor

roiLeo commented Jun 23, 2023

is there something that can be done about the repeating <template #trigger="{ active }"> ?

1. can we make trigger active by default

2. what are the other options except for active?

I think we can replace it by <template #trigger> and it would still work

@vikiival
Copy link
Member

cc @preschian ?

@preschian
Copy link
Member Author

preschian commented Jun 27, 2023

is there something that can be done about the repeating
<template #trigger="{ active }"> ?

feel free to create a wrapper component for this case. for NeoDropdown let it extends ODropdown components

  1. can we make trigger active by default
  2. what are the other options except for active?
  1. need to pass it down from slot-props. ref: https://oruga.io/components/Dropdown.html
  2. for now only active from slot-props. ref: https://github.com/oruga-ui/oruga/blob/126aba420d7b0635e8d1c9aec1202ab1bf2eab20/packages/oruga/src/components/dropdown/Dropdown.vue#L17

use #trigger={ active } when it needs active state, for example in this component, active state used on NeoButton props

<template #trigger="{ active }">
<NeoButton
:label="selectedTimeRange.label"
class="time-range-button"
no-shadow
:active="active" />
</template>

in this component, no need active state from dropdown. so, just write it #trigger

<template #trigger>
<NeoIcon icon="globe" />
<span>{{ $t('profileMenu.language') }}</span>
</template>

ref: https://vuejs.org/guide/components/slots.html#scoped-slots

@codeclimate
Copy link

codeclimate bot commented Jun 28, 2023

Code Climate has analyzed commit 71485d3 and detected 0 issues on this pull request.

View more on Code Climate.

@roiLeo roiLeo merged commit 2533ce3 into main Jun 28, 2023
18 of 19 checks passed
@roiLeo roiLeo deleted the refactor/neo-dropdown branch June 28, 2023 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
large Pull request is large S-code-lgtm-✅ code review guild has reviewed this PR and it's code is approved S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants