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

fix(web): Sort timezones in assets settings by offset #10697

Merged
merged 3 commits into from
Jul 1, 2024
Merged

fix(web): Sort timezones in assets settings by offset #10697

merged 3 commits into from
Jul 1, 2024

Conversation

WazWazTheDeveloper
Copy link
Contributor

fixes #10695

Copy link
Member

@bo0tzz bo0tzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One unnecessary nitpick, but looks good. Thanks!

Comment on lines 27 to 29
const timezones: ZoneOption[] = Intl.supportedValuesOf('timeZone')
.sort((zoneA, zoneB) => {
let numericallyCorrect = DateTime.local({ zone: zoneA }).offset - DateTime.local({ zone: zoneB }).offset;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do something like this so you're only calling DateTime.local() once:

  const timezones: ZoneOption[] = Intl.supportedValuesOf('timeZone')
  	.map((zone) => DateTime.local(zone))
    .sort((zoneA, zoneB) => {
      let numericallyCorrect = zoneA.offset - zoneB.offset;
      ...etc

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But you will have to pass the zone string for the creation of the list items in the last .map
so it will be something more like this

  const timezones: ZoneOption[] = Intl.supportedValuesOf('timeZone')
    .map((zone) => ({
      datatime: DateTime.local({ zone }),
      string: zone,
    }))
    .sort((zoneA, zoneB) => {
      let numericallyCorrect = zoneA.datatime.offset - zoneB.datatime.offset;
      if (numericallyCorrect != 0) {
        return numericallyCorrect;
      }
      const alphabeticallyCorrect = zoneA.string.localeCompare(zoneB.string, undefined, { sensitivity: 'base' });
      return alphabeticallyCorrect;
    })
    .map((zone) => ({
      label: zone.string + ` (${DateTime.local({ zone: zone.string }).toFormat('ZZ')})`,
      value: 'UTC' + DateTime.local({ zone: zone.string }).toFormat('ZZ'),
    }));

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DateTime.zoneName can be used:

const timezones: ZoneOption[] = Intl.supportedValuesOf('timeZone')
  .map((zone) => DateTime.local({ zone }))
  .sort((a, b) => a.offset - b.offset || a.zoneName.localeCompare(b.zoneName, undefined, { sensitivity: 'base' }))
  .map((date) => {
    const offset = date.toFormat('ZZ');
    return {
      label: `${date.zoneName} (${offset})`,
      value: 'UTC' + offset,
    };
  });

@alextran1502
Copy link
Contributor

Hello, can you add a unit test for this mechanism? Thank you for the contribution!

@alextran1502 alextran1502 merged commit 1d28285 into immich-app:main Jul 1, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timezones on web are sorted alphabetically
4 participants