Visualización de medios con HTML y JavaScript
Tengo un sitio web personal que utilizo para compartir videos e imágenes con amigos. A continuación se muestra un generador de medios que utiliza JavaScript y HTML. Su propósito principal es mostrar una imagen a la vez con solo hacer clic en un botón, pero se puede mostrar cualquier tipo de medio. Mi objetivo era crear una página de carga rápida para contener todos mis medios.
//VIDEO ARRAY
var oddvideo = [
'video1',
'video2',
'video3',
'video4',
'video5',
];
//AUDIO ARRAY
var oddaudio = [
'audio1',
'audio2',
'audio3',
'audio4',
'audio5',
];
//PHOTO ARRAY
var oddphoto = [
'photo1',
'photo2',
'photo3',
'photo4',
'photo5',
];
//TEXT ARRAY
var oddtext = [
'text1',
'text2',
'text3',
'text4',
'text5',
];
//RANDOM UNUSED ARRAY ITEMS
var Uvideo = [];
var Uaudio = [];
var Uphoto = [];
var Utext = [];
//OLD-NEW VARIABLES
var videoFor = 0;
var audioFor = 0;
var photoFor = 0;
var textFor = 0;
//NEW-OLD VARIABLES
var videoRev = oddvideo.length - 1;
var audioRev = oddaudio.length - 1;
var photoRev = oddphoto.length - 1;
var textRev = oddtext.length - 1;
//GENERATOR FUNCTION
function newThing() {
//RANDOM MODE
if(mode1.checked && (videoCheck.checked || audioCheck.checked || photoCheck.checked || textCheck.checked)) {
if (videoCheck.checked) {
if (!Uvideo.length) Uvideo = [...oddvideo];
var randomY = Uvideo;
}
if (audioCheck.checked) {
if (!Uaudio.length) Uaudio = [...oddaudio];
var randomY = Uaudio;
}
if (photoCheck.checked) {
if (!Uphoto.length) Uphoto = [...oddphoto];
var randomY = Uphoto;
}
if (textCheck.checked) {
if (!Utext.length) Utext = [...oddtext];
var randomY = Utext;
}
var randomX = Math.floor(Math.random() * (randomY.length));
var y = randomY;
var x = randomX;
document.getElementById("thingDisplay").innerHTML = y[x];
// remove randomx from the unused array since it's been used now
randomY.splice(randomX, 1);
}
//OLD-NEW MODE
if(mode2.checked && (videoCheck.checked || audioCheck.checked || photoCheck.checked || textCheck.checked)) {
if(videoCheck.checked) {
document.getElementById('thingDisplay').innerHTML = oddvideo[videoFor];
videoFor++;
if (videoFor >= oddvideo.length) videoFor = 0;
}
if(audioCheck.checked) {
document.getElementById('thingDisplay').innerHTML = oddaudio[audioFor];
audioFor++;
if (audioFor >= oddaudio.length) audioFor = 0;
}
if(photoCheck.checked) {
document.getElementById('thingDisplay').innerHTML = oddphoto[photoFor];
photoFor++;
if (photoFor >= oddphoto.length) photoFor = 0;
}
if(textCheck.checked) {
document.getElementById('thingDisplay').innerHTML = oddtext[textFor];
textFor++;
if (textFor >= oddtext.length) textFor = 0;
}
}
//NEW-OLD MODE
if(mode3.checked && (videoCheck.checked || audioCheck.checked || photoCheck.checked || textCheck.checked)) {
if(videoCheck.checked) {
document.getElementById('thingDisplay').innerHTML = oddvideo[videoRev];
videoRev--;
if (videoRev < 0) videoRev = oddvideo.length - 1;
}
if(audioCheck.checked) {
document.getElementById('thingDisplay').innerHTML = oddaudio[audioRev];
audioRev--;
if (audioRev < 0) audioRev = oddaudio.length - 1;
}
if(photoCheck.checked) {
document.getElementById('thingDisplay').innerHTML = oddphoto[photoRev];
photoRev--;
if (photoRev < 0) photoRev = oddphoto.length - 1;
}
if(textCheck.checked) {
document.getElementById('thingDisplay').innerHTML = oddtext[textRev];
textRev--;
if (textRev < 0) textRev = oddtext.length - 1;
}
}
}
<body>
<div align="center" id='thingDisplay'></div>
<div align="center">
<button onclick="newThing()">New Thing</button>
</div>
<form id="mode">
<label><input type="radio" name="modes" id="mode1"/></label> Random
<br/><label><input type="radio" name="modes" id="mode2"/></label> Old - New
<br/><label><input type="radio" name="modes" id="mode3"/></label> New - Old
</form>
<div align="right">
<form id="categories" align="right">
Video<label> <input type="radio" name="thing" id="videoCheck"/></label><br/>
Audio<label> <input type="radio" name="thing" id="audioCheck"/></label><br/>
Photo<label> <input type="radio" name="thing" id="photoCheck"/></label><br/>
Text<label> <input type="radio" name="thing" id="textCheck"/></label>
</form>
</div>
</body>
Algunas cosas a tener en cuenta ...
Organizo las matrices de JavaScript con la más joven en la parte superior y la más antigua en la parte inferior (con fechas, se vería así:
oddDate = ['Oct. 1', 'Oct. 2', 'Oct. 3', 'Oct. 4', 'Oct. 5'];El modo aleatorio es pseudoaleatorio y está diseñado para mostrar todos los elementos de la matriz una vez antes de repetir uno.
Los modos antiguo-nuevo y nuevo-antiguo se mueven a través de las matrices de arriba hacia abajo y de abajo hacia arriba , respectivamente.
Cada modo y categoría guarda su lugar cuando cambia a uno diferente. Por ejemplo, digamos que tiene el modo antiguo-nuevo activado y está en el elemento 3 de la categoría de texto. Cambia a la categoría de fotos, recorre un poco la matriz y luego regresa a la categoría de texto. Mostrará el siguiente elemento desde donde lo dejó anteriormente, es decir, el elemento 4 . Lo mismo puede decirse del cambio entre los tres modos; todos son independientes entre sí. Esto es algo que me gustaría mantener.
Una preocupación es que el generador y la página se volverán cada vez más lentos a medida que continúo agregando más elementos a las matrices, pero no estoy seguro de si esto es legítimo. En la versión en vivo, hay más de 500 elementos en cada matriz y se agregarán más con el tiempo.
Respuestas
Comentarios generales
Este código parece funcionar aceptablemente, aunque la semántica de "viejo - nuevo" parece invertida.
La primera viñeta de las notas dice:
- Organizo las matrices de JavaScript con la más joven en la parte superior y la más antigua en la parte inferior (con fechas, se vería así: oddDate = ['1 de octubre', '2 de octubre', '3 de octubre', '3 de octubre', 'Oct. 4 ',' 5 de octubre '];)
Sin embargo, la tercera viñeta dice:
- Los modos antiguo-nuevo y nuevo-antiguo se mueven a través de las matrices de arriba hacia abajo y de abajo hacia arriba , respectivamente.
Eso parece contradictorio, ya que si el más antiguo estaba en la parte inferior , la clasificación entre el antiguo y el nuevo debería comenzar al final del arreglo.
El código JavaScript parece muy repetitivo, especialmente en la función, newThing()pero también en los nombres de las variables. Recomendaría algunos cambios descritos más adelante, después de la revisión.
revisión
Javascript
Declaraciones de variables
El código usa algunas características específicas de ECMAScript-2015 (AKA ES-6) como la sintaxis de propagación . Con las variables de ES6 es recomendable utilizar consty letlimitar el alcance de las variables y evitar errores . Tenga en cuenta que eso constno significa inmutabilidad, sino que dicha variable no se puede reasignar.
Variables globales
El código hace referencia a elementos DOM utilizando las variables creadas implícitamente a partir de los atributos de id , p. Ej.
if(mode1.checked && (videoCheck.checked || audioCheck.checked || photoCheck.checked || textCheck.checked)) {
Si desea realizar una prueba unitaria del código JavaScript, esto podría dificultarlo.
Etiquetas de entrada
Parece que todas las entradas de radio están contenidas en <label>entradas, lo cual es bueno para la accesibilidad (por ejemplo, lectores de pantalla, capacidad del usuario para hacer clic en la etiqueta para enfocar / activar la entrada) sin embargo, el texto al lado de cada entrada no está dentro de la etiqueta. Parece inútil tener una etiqueta si la entrada es lo único que contiene.
HTML
Inline event handlers
The code sets up event handlers within the HTML code:
<button onclick="newThing()">New Thing</button>
It is better to register event handlers within the JavaScript (e.g. using newButton.addEventListener for multiple reasons:
- The logic can be separated from the markup - if multiple teammates worked on the project then one could work on the JavaScript while the other could work on the HTML independently.
- Such handlers can pollute the global namespace which can lead to strange behavior.
Aligning child elements
The <div> elements have align="center" and align="right". That appears to be a deprecated HTML 4 attribute and no longer supported in HTML 5. This can be converted to CSS rules. Note that text-align applies to block containers1. To center inline elements like images, video, etc. the display would need to be set to block and margin set to auto.
Multiple form elements
There are two separate <form> elements. The two could be combined into a single form that contains all elements.
Simplifying code
The following changes, along with suggestions from the review points above can be used to dramatically decrease the length of the code:
put options into an object that can contain the current index, and make the object a property of an object where the property name (i.e. key) is the type of thing - e.g.
const options = { video: { options: [ //VIDEO ARRAY 'video1', 'video2', 'video3', 'video4', 'video5', ], currentIndex: -1 }, audio: { options: [ //AUDIO ARRAY 'audio1', 'audio2', 'audio3', 'audio4', 'audio5', ], currentIndex: -1 }, photo: { options: [ //PHOTO ARRAY 'photo1', 'photo2', 'photo3', 'photo4', 'photo5', ], currentIndex: -1 }, text: { options: [ //TEXT ARRAY 'text1', 'text2', 'text3', 'text4', 'text5', ], currentIndex: -1 }, }instead of using
idattributes for the radio buttons, just give themvalueattributesreference the form elements via
document.forms.elementsdetermine which radio buttons are selected via RadioNodeList.value
use an element for the display of the thing.
use a class to encapsulate the indexes, along with methods to get an item based on the mode
class ThingList {
constructor(items) {
this.items = items;
this.randomUnused = [...items];
this.forwardIndex = 0;
this.reverseIndex = items.length - 1;
}
forwardItem() {
return this.items[this.forwardIndex++ % (this.items.length)];
}
randomItem() {
if (!this.randomUnused.length) {
this.randomUnused.push(...this.items);
}
const index = Math.floor(Math.random() * this.randomUnused.length)
return this.randomUnused.splice(index, 1);
}
reverseItem() {
if (this.reverseIndex < 0) {
this.reverseIndex = this.items.length - 1;
}
return this.items[this.reverseIndex--];
}
}
const options = {
video: new ThingList([ //VIDEO ARRAY
'video1',
'video2',
'video3',
'video4',
'video5',
]),
audio: new ThingList([ //AUDIO ARRAY
'audio1',
'audio2',
'audio3',
'audio4',
'audio5',
]),
photo: new ThingList([ //PHOTO ARRAY
'photo1',
'photo2',
'photo3',
'photo4',
'photo5',
]),
text: new ThingList([ //TEXT ARRAY
'text1',
'text2',
'text3',
'text4',
'text5',
])
}
const output = document.getElementsByTagName('output')[0];
//GENERATOR FUNCTION
function newThing() {
if (!(document.forms.thingSelection.type.value in options)) {
return false;
}
const list = options[document.forms.thingSelection.type.value];
const method = document.forms.thingSelection.mode.value + 'Item';
const item = list[method]();
output.innerHTML = item;
}
document.getElementsByTagName('button')[0].addEventListener('click', newThing)
.center {
text-align: center;
}
.right {
text-align: right;
}
<div class="center"><output></output></div>
<div class="center">
<button>New Thing</button>
</div>
<form name="thingSelection">
<label><input type="radio" name="mode" value="random" /> Random</label>
<br /><label><input type="radio" name="mode" value="forward" /> Old - New</label>
<br /><label><input type="radio" name="mode" value="reverse" /> New - Old</label>
<div class="right">
<label>Video <input type="radio" name="type" value="video" /></label><br />
<label>Audio <input type="radio" name="type" value="audio" /></label><br />
<label>Photo <input type="radio" name="type" value="photo" /></label><br />
<label>Text <input type="radio" name="type" value="text" /></label>
</div>
</form>