การแสดงสื่อด้วย HTML และ JavaScript

Question

ฉันมีเว็บไซต์ส่วนตัวที่ใช้แชร์วิดีโอและรูปภาพกับเพื่อน ๆ ด้านล่างนี้คือตัวสร้างสื่อโดยใช้ JavaScript และ HTML จุดประสงค์หลักคือการแสดงทีละภาพด้วยการคลิกปุ่ม แต่สามารถแสดงสื่อประเภทใดก็ได้ เป้าหมายของฉันคือสร้างหน้าที่โหลดเร็วเพื่อเก็บสื่อทั้งหมดของฉัน

//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>&nbsp;Random
    <br/><label><input type="radio" name="modes" id="mode2"/></label>&nbsp;Old&nbsp;-&nbsp;New
    <br/><label><input type="radio" name="modes" id="mode3"/></label>&nbsp;New&nbsp;-&nbsp;Old
  </form>

  <div align="right">
    <form id="categories" align="right">
      Video<label>&nbsp;<input type="radio" name="thing" id="videoCheck"/></label><br/>
      Audio<label>&nbsp;<input type="radio" name="thing" id="audioCheck"/></label><br/>
      Photo<label>&nbsp;<input type="radio" name="thing" id="photoCheck"/></label><br/>
      Text<label>&nbsp;<input type="radio" name="thing" id="textCheck"/></label>
    </form>
  </div>
</body>

บางสิ่งที่ควรทราบ ...

• ฉันจัดระเบียบอาร์เรย์ JavaScript โดยให้อายุน้อยที่สุดอยู่ด้านบนสุดและเก่าแก่ที่สุดที่ด้านล่าง (โดยมีวันที่จะมีลักษณะดังนี้:

  oddDate = ['Oct. 1', 'Oct. 2', 'Oct. 3', 'Oct. 4', 'Oct. 5']; 
  

• โหมดสุ่มเป็นแบบสุ่มหลอกและออกแบบมาเพื่อแสดงรายการอาร์เรย์ทั้งหมดหนึ่งครั้งก่อนที่จะทำซ้ำ

• โหมดเก่า - ใหม่และเก่าจะย้ายผ่านอาร์เรย์จากบนลงล่างและล่างขึ้นบนตามลำดับ

• แต่ละโหมดและหมวดหมู่จะบันทึกตำแหน่งเมื่อคุณเปลี่ยนเป็นโหมดอื่น ตัวอย่างเช่นสมมติว่าคุณเปิดโหมดเก่า - ใหม่และอยู่ในรายการ 3ของหมวดหมู่ข้อความ คุณเปลี่ยนไปใช้หมวดหมู่ภาพถ่ายวิ่งผ่านอาร์เรย์เล็กน้อยจากนั้นกลับไปที่หมวดหมู่ข้อความ มันจะแสดงรายการถัดไปจากที่คุณซ้ายปิดก่อนหน้านี้ - คือข้อ 4 เช่นเดียวกันกับการสลับไปมาระหว่างโหมดทั้งสาม พวกเขาทั้งหมดเป็นอิสระจากกัน นี่คือสิ่งที่ฉันอยากจะรักษา

• ข้อกังวลอย่างหนึ่งคือตัวสร้างและเพจจะช้าลงและช้าลงเมื่อฉันเพิ่มรายการเพิ่มเติมในอาร์เรย์ต่อไป แต่ฉันไม่แน่ใจว่าสิ่งนี้ถูกต้องหรือไม่ ในเวอร์ชันที่ใช้งานจริงมีรายการมากกว่า 500 รายการในแต่ละอาร์เรย์และจะมีการเพิ่มอีกมากมายเมื่อเวลาผ่านไป

Answer 1

ข้อเสนอแนะโดยรวม

ดูเหมือนว่ารหัสนี้จะใช้งานได้ดีแม้ว่าความหมายของ "เก่า - ใหม่" จะกลับกัน

สัญลักษณ์แสดงหัวข้อย่อยแรกในหมายเหตุระบุ:

• ฉันจัดระเบียบอาร์เรย์ JavaScript โดยให้อายุน้อยที่สุดอยู่บนสุดและเก่าแก่ที่สุดอยู่ด้านล่างสุด (วันที่จะมีลักษณะดังนี้: oddDate = ['1 ต.ค. ', '2 ต.ค. ', '3 ต.ค. ', "ต.ค. 4 ',' 5 ต.ค. '];)

ยังมีสัญลักษณ์แสดงหัวข้อย่อยที่สาม:

• โหมดเก่า - ใหม่และเก่าจะย้ายผ่านอาร์เรย์จากบนลงล่างและล่างขึ้นบนตามลำดับ

ดูเหมือนจะขัดแย้งกันเนื่องจากถ้าเก่าที่สุดอยู่ที่ด้านล่างการเรียงลำดับแบบเก่า - ใหม่ควรเริ่มต้นที่ส่วนท้ายของอาร์เรย์

โค้ด JavaScript ดูเหมือนซ้ำซากมาก - โดยเฉพาะในฟังก์ชันnewThing()แต่ยังรวมถึงชื่อตัวแปรด้วย ฉันขอแนะนำการเปลี่ยนแปลงบางอย่างที่อธิบายไว้ในภายหลังหลังจากการตรวจสอบ

ทบทวน

Javascript

การประกาศตัวแปร

รหัสใช้คุณสมบัติบางอย่างที่เฉพาะเจาะจงเพื่อECMAScript ปี 2015 (AKA ES-6) เช่นไวยากรณ์การแพร่กระจาย กับตัวแปร ES6 ก็จะแนะนำให้ใช้constและletเพื่อ จำกัด ขอบเขตของตัวแปรและข้อบกพร่องหลีกเลี่ยง โปรดทราบconstว่าไม่ได้หมายถึงความไม่เปลี่ยนรูป แต่ไม่สามารถกำหนดตัวแปรดังกล่าวใหม่ได้

ตัวแปรส่วนกลาง

รหัสอ้างอิงองค์ประกอบ DOM โดยใช้ตัวแปรที่สร้างขึ้นโดยปริยายจากแอตทริบิวต์id - เช่น

if(mode1.checked && (videoCheck.checked || audioCheck.checked || photoCheck.checked || textCheck.checked)) {

หากคุณต้องการทดสอบโค้ด JavaScript ของหน่วยการดำเนินการนี้อาจทำให้ยาก

ใส่ป้ายกำกับ

ดูเหมือนว่าอินพุตวิทยุทั้งหมดมี<label>อินพุตซึ่งเหมาะสำหรับการเข้าถึง (เช่นโปรแกรมอ่านหน้าจอความสามารถของผู้ใช้ในการคลิกฉลากเพื่อโฟกัส / เปิดใช้งานอินพุต) แต่ข้อความที่อยู่ถัดจากอินพุตแต่ละรายการไม่อยู่ในป้ายกำกับ ดูเหมือนไม่มีประโยชน์ที่จะมีป้ายกำกับหากอินพุตเป็นสิ่งเดียวในนั้น

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:

1. 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.
2. 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 containers¹. 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 id attributes for the radio buttons, just give them value attributes

• reference the form elements via document.forms.elements

• determine 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" />&nbsp;Random</label>
  <br /><label><input type="radio" name="mode" value="forward" />&nbsp;Old&nbsp;-&nbsp;New</label>
  <br /><label><input type="radio" name="mode" value="reverse" />&nbsp;New&nbsp;-&nbsp;Old</label>
  <div class="right">
    <label>Video&nbsp;<input type="radio" name="type" value="video" /></label><br />
    <label>Audio&nbsp;<input type="radio" name="type" value="audio" /></label><br />
    <label>Photo&nbsp;<input type="radio" name="type" value="photo" /></label><br />
    <label>Text&nbsp;<input type="radio" name="type" value="text" /></label>
  </div>
</form>