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

Nautical mile option in Units-Editor #1059

Merged
merged 7 commits into from
Mar 26, 2024

Conversation

Avengium
Copy link
Contributor

@Avengium Avengium commented Mar 16, 2024

Description

I added nautical mile as an option for Units Editor. In Distance --> Distance Unit.

Type of change

New option in dropdown distance unit.

Versioning

  • Version not updated
  • Changed files hash not updated

In Distance --> Distance Unit.
Copy link

netlify bot commented Mar 16, 2024

Deploy Preview for afmg ready!

Name Link
🔨 Latest commit d2bf080
🔍 Latest deploy log https://app.netlify.com/sites/afmg/deploys/66031959ba4fd00008df66ef
😎 Deploy Preview https://deploy-preview-1059--afmg.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 configuration.

@Avengium
Copy link
Contributor Author

The "league" unit in distance has the numbers of a nautical league.
If Azgaar want to clarify the current one, already on the program has the numbers of a nautical league, the text could be updated to:

    function toKilometer(v) {
      if (unit === "km") return v;
      else if (unit === "mi") return v * 1.60934;
      else if (unit === "lg") return v * 4.828;
      else if (unit === "vr") return v * 1.0668;
      else if (unit === "nmi") return v * 1.852;
      else if (unit === "nlg") return v * 5.556;
      return 0; // 0 if distanceUnitInput is a custom unit
    }

and the HTML like:

          <div data-tip="Select a distance unit or provide a custom name">
            <div>Distance unit:</div>
            <select id="distanceUnitInput" data-stored="distanceUnit">
              <option value="mi" selected>Mile (mi)</option>
              <option value="km">Kilometer (km)</option>
              <option value="lg">League (lg)</option>
              <option value="vr">Versta (vr)</option>
              <option value="nmi">Nautical mile (nmi)</option>
              <option value="nlg">Nautical league (nlg)</option>
              <option value="custom_name">Custom name</option>
            </select>
          </div>

@StempunkDev
Copy link
Contributor

The "league" unit in distance has the numbers of a nautical league. If Azgaar want to clarify the current one, already on the program has the numbers of a nautical league, the text could be updated to:

    function toKilometer(v) {
      if (unit === "km") return v;
      else if (unit === "mi") return v * 1.60934;
      else if (unit === "lg") return v * 4.828;
      else if (unit === "vr") return v * 1.0668;
      else if (unit === "nmi") return v * 1.852;
      else if (unit === "nlg") return v * 5.556;
      return 0; // 0 if distanceUnitInput is a custom unit
    }

I would like to suggest the following refactor of the toKilometer Function for possibile expansions in the future :

    function toKilometer(v) {
      switch(unit) {
      case 'km': return v;
      case 'mi': return v * 1.60934;
      case 'lg': return v * 4.828;
      case 'vr': return v * 1.0668;
      case 'nmi': return v * 1.852;
      case 'nlg': return v * 5.556;
      default: return 0; // 0 if distanceUnitInput is a custom unit
     }
    }

No break; is needed as we return in every line. I would say this is easier to read.

@Azgaar Azgaar assigned Azgaar and Avengium and unassigned Azgaar Mar 16, 2024
@Azgaar
Copy link
Owner

Azgaar commented Mar 16, 2024

I would like to suggest the following refactor of the toKilometer Function for possibile expansions in the future

I don't see how it's better. If with earlier exit can be simpler than switch.

@Avengium
Copy link
Contributor Author

Avengium commented Mar 19, 2024

Azgaar, what's your decision on this added code?
Current commit only has nautical mile and proposed edits on comments are:

  • Change current league to nautical league as 5.556 appears to be nautical.
  • Create league with 4.828 as conversion factor for land league.

Also, do we put the switch or the list of else ifs?

@Azgaar
Copy link
Owner

Azgaar commented Mar 20, 2024

It's fine, you can add. The code should be like that:

    function toKilometer(v) {
      if (unit === "km") return v;
      if (unit === "mi") return v * 1.60934;
      if (unit === "lg") return v * 5.556;
      if (unit === "vr") return v * 1.0668;
      return 0;
    }

Else is not required as we are returning value immediately.

The distance 5.556 km = 1 league, is put on the nautical league and used the English land league 4.828 to differentiate both units.
@Azgaar
Copy link
Owner

Azgaar commented Mar 26, 2024

I'm ok to merge, please update the version

changed 1.97.02 to 1.97.03
@Azgaar
Copy link
Owner

Azgaar commented Mar 26, 2024

@Avengium, hm, not that way. Now there is a conflict. Also need to update hash for changed scripts in index.html.

versioning.js Outdated Show resolved Hide resolved
@Azgaar Azgaar merged commit 73e9d22 into Azgaar:master Mar 26, 2024
@Avengium Avengium deleted the Units-Editor-DistanceUnit branch April 4, 2024 18:33
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.

3 participants