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

✨ NeoDropdown & NeoSelect #4430

Merged
merged 12 commits into from
Dec 13, 2022
Merged

Conversation

roiLeo
Copy link
Contributor

@roiLeo roiLeo commented Nov 30, 2022

// TODO:

  • dark-mode for NeoSelect & NeoDropdown
  • font-awesome integration in histoire
  • check oruga scss global var
  • active style
  • new props/slot
  • responsive dropdown (mobile version)

PR Type

  • Feature

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

Screenshot 📸

Screenshot 2022-11-30 at 14-27-54 NeoDropdown › With Button Histoire

@netlify
Copy link

netlify bot commented Nov 30, 2022

Deploy Preview for koda-nuxt ready!

Name Link
🔨 Latest commit f279921
🔍 Latest deploy log https://app.netlify.com/sites/koda-nuxt/deploys/6396fa98c545f70009e51583
😎 Deploy Preview https://deploy-preview-4430--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.

@roiLeo roiLeo self-assigned this Dec 6, 2022
@roiLeo roiLeo marked this pull request as ready for review December 7, 2022 13:30
@roiLeo roiLeo requested a review from a team as a code owner December 7, 2022 13:30
@roiLeo roiLeo requested review from preschian and removed request for a team December 7, 2022 13:30
@yangwao
Copy link
Member

yangwao commented Dec 8, 2022

Awesome stuff! 😍

Is it mergeable already?

@roiLeo
Copy link
Contributor Author

roiLeo commented Dec 8, 2022

Awesome stuff! 😍

Is it mergeable already?

ještě ne
Screenshot 2022-12-08 at 12-48-42 KodaDot - Kusama NFT Market Explorer

@roiLeo roiLeo marked this pull request as draft December 8, 2022 12:08
@roiLeo roiLeo mentioned this pull request Dec 8, 2022
18 tasks
@yangwao
Copy link
Member

yangwao commented Dec 8, 2022

ještě ne

J'ai hâte qu'il soit prêt pour que nous puissions l'utiliser 🥰

@roiLeo
Copy link
Contributor Author

roiLeo commented Dec 8, 2022

This PR break current oruga tabs design, I'll be looking why

Screenshot 2022-12-08 at 14-56-59 KodaDot - Kusama NFT Market Explorer

@preschian
Copy link
Member

preschian commented Dec 12, 2022

This PR break current oruga tabs design, I'll be looking why

oh, I found it. need to add scoped on every histoire component will fix this issue https://github.com/roiLeo/nft-gallery/blob/feature/ui/components/libs/ui/src/components/NeoDropdown/NeoDropdown.vue#L27

somehow if we didn't put the scoped attributes these components will be broken

// OVERRIDE ORUGA COMPONENTS
@import "components/o-table";
@import "components/o-tabs";
@import "components/o-tooltip";

libs/ui/src/histoire.css Outdated Show resolved Hide resolved
@roiLeo
Copy link
Contributor Author

roiLeo commented Dec 12, 2022

somehow if we didn't put the scoped attributes these components will be broken

I remember mentioning it in a comment, wanted to know if there was not a better approach like a general import.

now my problem is that I can't override oruga variable on scoped element using :root selector

@preschian
Copy link
Member

now my problem is that I can't override oruga variable on scoped element using :root selector

i see, what hapeen if we try with sass variables? does the sass variable also get the same result?

@roiLeo
Copy link
Contributor Author

roiLeo commented Dec 12, 2022

i see, what hapeen if we try with sass variables? does the sass variable also get the same result?

Sass variable ain't working :/

edit: might need some tweak

In order to work with SASS/SCSS you might also have to install sass and sass-loader depending on your environment.

@preschian
Copy link
Member

preschian commented Dec 12, 2022

Sass variable ain't working :/

we need to reorder like this:

@import '@oruga-ui/oruga/src/scss/utilities/expressions.scss';
@import '@oruga-ui/oruga/src/scss/utilities/variables.scss';
@import '@oruga-ui/oruga/src/scss/utilities/animations.scss';
@import '@oruga-ui/oruga/src/scss/utilities/helpers.scss';

$button-background-color: red !default;
$button-color: black !default;
// override another variables in here

@import '@oruga-ui/oruga/src/scss/components/button.scss';

// custom classes in here

should work with scoped components. the order was:

  1. their utilities/mixins
  2. override their variables
  3. single component
  4. last, override with classes in case needed

wrong order will won't work as expected

@roiLeo roiLeo marked this pull request as ready for review December 12, 2022 09:47
@roiLeo roiLeo changed the title ✨ wip: NeoDropdown & NeoSelect ✨ NeoDropdown & NeoSelect Dec 12, 2022
@codeclimate
Copy link

codeclimate bot commented Dec 12, 2022

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

View more on Code Climate.

@roiLeo
Copy link
Contributor Author

roiLeo commented Dec 12, 2022

Thanks for your help! I'm not really a fan of the current solution, but it look like it's working (don't work when using scoped scss in NeoDropdown component)

@preschian
Copy link
Member

Thanks for your help! I'm not really a fan of the current solution, but it look like it's working (don't work when using scoped scss in NeoDropdown component)

yes, would be happy if you found a cleaner solution. that's my approach in the past when I'm wrapping https://ant.design/ components

@yangwao
Copy link
Member

yangwao commented Dec 12, 2022

I'm not really a fan of the current solution

what would be better solution?

@roiLeo
Copy link
Contributor Author

roiLeo commented Dec 12, 2022

what would be better solution?

IMO there must be a better way to configure this stuff.
We could try to create a single file outside histoire components in which we override some sass variable. (like this)

otherwise we should really stop using oruga outside libs/ui folder

@yangwao
Copy link
Member

yangwao commented Dec 12, 2022

oh okay, for now we can merge then?

@roiLeo
Copy link
Contributor Author

roiLeo commented Dec 12, 2022

oh okay, for now we can merge then?

I think it's good to go, don't mind failing test

@yangwao yangwao mentioned this pull request Dec 12, 2022
@yangwao
Copy link
Member

yangwao commented Dec 13, 2022

let's see what we can do with this

@yangwao yangwao merged commit dc6b5ea into kodadot:main Dec 13, 2022
@roiLeo roiLeo deleted the feature/ui/components branch December 13, 2022 12:24
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.

Create NeoSelect create NeoDropdown component based on o-dropdown in histoire
3 participants