Drum Machine hot or not?

Drum Machine hot or not?
0

#1

I’m mostly looking for opinion if my project is “react enough”, i.e something isn’t done by react standards, but any feedback is greatly appreciated in advance.
Here’s a pen.


#2

If you have an on and off button, then I would think the buttons should not light up as if they were on.

Not React specific, but those two switch statements with all of the almost identical code should be refactored using some kind of object or array.

Also, making all those manual refs. What if you had 100 sound banks for the 9 keys, think about how you could better manage the refs.

You could definitely refactor both the handleVolume and onOff methods to a single setState in each instead of having two very similar setState calls.

With the exception of the audio elements, I am not sure why you are using refs for the OutDiv component. You should only use refs when there is no other way.


#3

To compare it with element being clicked. If some element is being clicked, then corresponding sound is being played. Should i just hardcoded them?Example:

 switch (e.target.id) {
			case "Heater-1":
            this.sounds.sound1.volume = this.state.volume;
            this.sounds.sound1.play(); 
            if(this.state.volume===0){
		       displayId = null;
	        }
	        else{
	           displayId = e.target.id; 
		    }	
		    break;
rest of code****

Or should i just outright, make arrays/object containing all ids and loop through them?

When your device is off, you can despite that increase volume, thus when you turn back on, it remembers that increased value. And on the slider side, you can increase volume i.e. move switch despite it’s turned off.

Light buttons when is turned of, ok, i’ll do change that.

About manual refs, not sure what you talking about … See, i’m new to React and at you suggestion in previous post, i looked at documentation … Is this some advanced technique? I’m just curious, don’t know any better.

Question: Should i use just for example this.H1.current as a “hook” of sorts or ReactDOM.findDOMNode(this.H1.current) which i saw somewhere that it need to be mounted/dismounted as event listeners do? Both produce same results:

console.log(this.H1.current.id);

and

console.log(ReactDOM.findDOMNode(this.H1.current).id);

Both of them in console display id.
Edit1: When i click with mouse, i don’t actually click on audio element, i click on it’s parent. So i must either compare it with harcoded id or with id taken from some array which position corresponds to it index i.e. position of case statement. Either way, it’s hardcoding. Long story/short: So i should either hardcode refs or id’s?

Edit2: I could use

.parentNode.id

on audio ref to get id which is later displayed in display component. But should i? Not sure if it’s this count towards as “directly manipulating dom”?


#4

I think i managed to “repair” everything you suggested. Care to take a look again?


#5

Definitely looks better. You seem to have a lot of comments which explain what the code is doing. It is a matter of opinion, but I prefer to only see comments which explain why a particular line was written vs describing what the code is doing. My point is unless you are writing some kind of syntax which not typical, the code should speak for itself. Your naming conventions see good enough most of the time to avoid most of the comments you have written. Again, this is more of a personal preference. It is worth nothing though, it make the overall code hard to read when all of the comments are scattered through a class or function. May try putting a summary of inputs and outputs to the function and their purpose at the beginning or write before the class or function if you feel the need to explain what the code does. Any decent programmer will understand what the code is doing, but may not always understand your choices for how certain variables are initializationed. I find sometimes those are good places to explain why you chose certain values over others if it might cause confusion reading the code.

Anyway, enough about comments. I do have one more suggestion. With my drum machine (though not React), does allow the user to click or hit the keys quickly to play the same sound again before it finishes playing. I am sure you can do the same with your React version. Those chord sounds are the most noticeable. I should be able to click those chord keys 5 times in a row very quickly and hear the sound start each time.


#6

Cool! That takes burden from my heart.
Second thing: I think it’s gotta do of how long sound it is. For example, sound with id “Heater-1” have about quarter of its timeline a silence at the end. So i either crop the silence or stop it playing(sound) whenever is subsequently is clicked. Since it’s hosted outside my reach, i guess i should proceed with latter?


#7

Done.
Before:

playAudio(par1, par2, par3){//Par1 is index of an given element be that be audio or outer div. Par2 is this.state.volume and Par3 is element itself.
		
    //Assign par1 as index to know which element(by ref) should 'engage'.
    sounds[par1].volume = par2;//After element is 'engaged', set it's volume.
    sounds[par1].play();//After element is 'engaged', set it's to play.
			
	return arguments;
}

After:

playAudio(par1, par2, par3){//Par1 is index of an given element be that be audio or outer div. Par2 is this.state.volume and Par3 is element itself.
    //Assign par1 as index to know which element(by ref) should 'engage'.
    sounds[par1].volume = par2;//After element is 'engaged', set it's volume.
    sounds[par1].currentTime = 0;//Added this so you may for example, play sound 5 times subsequently.
    sounds[par1].play();//After element is 'engaged', set it's to play.
			
    return arguments;
}

#8

I also added an loop for refs, so if need adding more refs for whatever reason, person should just add them in said array.

//Constructor and it's things ...
	//An array of all refs of audio elements.
		this.audioRefs = [];
		
		for(let elem in this.pads.reff){
			
			this[elem] = React.createRef();
			this.audioRefs.push(this[elem]);
			
		}
		
	}

componentDidMount() {
		//Set event listeners for keypress
		window.addEventListener("keyup", this.keyPress);
		
		//Gather all audio objects through their respective refs.		
		for(let elem in this.audioRefs){
			
			sounds.push(ReactDOM.findDOMNode(this[elem].current));
			
		}
		
	}

Now for componentWillUnmount() lifecycle method there’s no mention of disposing refs in documentation … In fact, i didn’t find anywhere how to dispose an array of disposable elements so to speak … That in fact if i need to explicitly dispose refs? Also in refs in documentation there is mounting of refs but there is no unmounting. Should i just “not unmount them”? Edit1: I also have global variable array “sounds” n which i put created refs. Should i use global in this case? Declaring constants will not do, nor declaring as “this.sounds = [];” in constructor. It says undefined … There;s also possibility to setState and not using this global. But should i do that? Should setState instead using global variable sounds? Edit2: Discard what i’m said about setState. It doesn’t work … Still … is there a alternative to using global variable in this case? Edit3: I also browsed again through refs documentation and didn’t find sign of use of “findDOMNode” in connection with refs, at least not in my example to play audios, so that’s a mistake from my side.


#9

Well, i solved my problem concerning variable saying “undefined”. It’s because i didn’t used and arrow function inside a setTimeout and because of that and don’t have to use global variable called “sounds” anymore. And after sifting through various posts and documentation itself,

isn’t required(in my case) for use with refs.
Summary: There’s no “quirks” anymore, just one more final check and it of it goes.