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

Consider making "className" dynamic #8655

Closed
noxecane opened this issue Aug 18, 2019 · 3 comments
Closed

Consider making "className" dynamic #8655

noxecane opened this issue Aug 18, 2019 · 3 comments

Comments

@noxecane
Copy link

Motivation

Issue #6299 made a case to add className to popup options, hence making it possible to style the entire popup. I'm in a position where that style changes dynamically every second or so, and would want to update the specific popup to reflect the new style.

Suggestions

  1. A method added to the Popup class, toggleClass that will give me the ability remove/add a class dynamically since className is basically a proxy to classList.add
  2. A method added to the Popup class, setClassName that will give me the ability where I update all the classes in one call

Mockup

const myPop = new Popup({ className: 'rounded offline' }).setText('This is my home')

// .....in some evented scenario for suggestion-1
listener.on('new-event', (x) => {
  x.toggleClass('is-open')
})

// .....same scenario for suggestion-2 with more context
listener.on('new-event', (x) => {
  x.setClassName(x.online ? 'rounded online' : 'rounded offline')
})

I would prefer the second one as it fits my context the most.

@ahk
Copy link
Contributor

ahk commented Aug 23, 2019

I think both options are fine though I think the toggleClass would see more general usage. I'd be inclined to accept both of these suggestions.

Would you have any interest in making a pull request for this? I'd be happy to help get it merged. If not I may be able to fit this in next week.

@Ashot-KR
Copy link
Contributor

Ashot-KR commented Sep 16, 2019

@ahk here you go #8759. Description in PR

@noxecane actually you can do it without any changes of Popup class, but in a little bit verbose way:

const popup = new Popup()

// access `classList` property of popup instance container:
const { classList } = popup.getElement()

// use .add, .remove, .toggle of ClassList
classList.add('new-class')
classList.remove('new-class')
classList.toggle('toggle-class')

@noxecane
Copy link
Author

Wow thanks...just saw this....26 days ago 🙆🏽‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants