Add support for system-ui fallbacks

[onigoetz]

The system-ui font family is a special font family that will be changed to the platform's default font family.

This feature will add fallback font families when unsupported

.foo {
  font-family: Helvetica, system-ui, sans-serif;
}

.bar {
  font: 100%/1.5 Helvetica, system-ui, sans-serif;
}

becomes

.foo {
  font-family: Helvetica, system-ui, AppleSystem, BlinkMacSystemFont, Segoe UI, Roboto, Noto Sans, Ubuntu, Cantarell, Helvetica Neue, sans-serif;
}

.bar {
  font: 100% / 1.5 Helvetica, system-ui, AppleSystem, BlinkMacSystemFont, Segoe UI, Roboto, Noto Sans, Ubuntu, Cantarell, Helvetica Neue, sans-serif;
}

The list of fonts comes from this postcss plugin: https://www.npmjs.com/package/postcss-font-family-system-ui

These are my very first lines of Rust, I'm open to feedback if something isn't idiomatic or could be made more efficiently or anything really :)

[onigoetz]

I changed these lines to be the same as what the output already contains

[devongovett]

Thanks for the PR! A few comments that might help make it a bit more idiomatic Rust, but what you did was by no means wrong.

Would you also mind adding a test? lib.rs has most of them. It's pretty long but you could add a test to the test_font function in there.

[devongovett]

You could avoid the unwraps here, and the duplicate contains/position check with something like this:

if let Some(families) = &mut family {
  if let Some(position) = families.iter().position(|v| *v == system_ui) {
    families.splice(
      (position + 1)..(position + 1),
      DEFAULT_SYSTEM_FONTS
        .iter()
        .map(|name| FontFamily::FamilyName(CowArcStr::from(*name))),
    );
  }
}

You'll need to add mut to the definition of family in the function arguments for this to work

[onigoetz]

Truly a fascinating language, thanks, I'll try that construct next time

[devongovett]

With the suggestion below, this could be a const of strings instead:

const DEFAULT_SYSTEM_FONTS: &[&str] = &[
  // #1: Supported as the -apple-system value (only on Mac)
  "AppleSystem",
  // #2: Supported as the 'BlinkMacSystemFont' value (only on Mac)
  "BlinkMacSystemFont",
  "Segoe UI",  // Windows >= Vista
  "Roboto",    // Android >= 4
  "Noto Sans", // Plasma >= 5.5
  "Ubuntu",    // Ubuntu >= 10.10
  "Cantarell", // GNOME >= 3
  "Helvetica Neue",
];

[onigoetz]

done. I tried to extract it before but by keeping the FontFamily and the compiler told me that CowArcStr could not be in a static context. looks better indeed

[devongovett]

Could be a const

const SYSTEM_UI: FontFamily = FontFamily::Generic(GenericFontFamily::SystemUI);

[onigoetz]

done

[devongovett]

You could either return early here if is_supported is true, or not even call this function in that case. Either way it would avoid some extra checks.

[onigoetz]

I chose the early return

[onigoetz]

Thanks for the feedback, will do that tonight

Indeed I can add tests, forgot to ask you where to put them.

[devongovett]

I guess system-ui should be first?

[onigoetz]

I put it second on purpose, this makes sure that the fonts are added at the right place and don't remove the existing. Also, not everybody has Helvetica so it could make sense to want Helvetica but fallback to whatever system font the person has.

[devongovett]

Ah, sorry, I misread the input. Thanks!