Improvements to About

[edfloreshz]

This PR makes several improvements to the about section.

• Per @wiiznokes suggestion in Feedback on About feature #696, the about section code was moved to a widget.
• Simplifies builder methods.
• Properties are now private.
• Removed the URL specific methods and made method links public, letting anyone pass an array of label/link pairs.
• Added the license crate to handle licenses.
• Removed license_type and replaced it with a link to the license page (https://spdx.org/licenses).
• Add spacing between content and scrollbar.

Closes #696

[mmstick]

Some tests are failing with an unresolved import for about

[edfloreshz]

Sorry, should be fixed.

[wiiznokes]

some nitpicks:

• would be nice if the fields were private, and the set_ prefix were removed.
• some data are duplicated. For example, repository_url is stored in a dedicated field and the links BtreeMap. I can see the value of having dedicated fields, for example, to add custom icons in the future
• would be nice to add a method add_link(self, name, link) -> Self
• set_support_url is ambigus imo, for example, i have this value set in metainfo.xml: bugtracker, donation, vcs-browser, homepage
• maybe switch license with license_type, and rename license_type to licence_text

wdyt @edfloreshz ?

[edfloreshz]

Those are great suggestions, I'll make the changes in the next couple of hours.

[wiiznokes]

here are some ideas of urls we can add https://docs.flathub.org/docs/for-app-authors/metainfo-guidelines/#url

[edfloreshz]

• Properties are now private.
• Removed the URL specific methods and made method links public, letting anyone pass an array of label/link pairs.
• Added the license crate to handle licenses.
• Removed license_type and replaced it with a link to the license page (https://spdx.org/licenses).

This is what it looks like now:

About::default()
  .name("About Demo")
  .icon(Self::APP_ID)
  .version("0.1.0")
  .author("System 76")
  .license("GPL-3.0-only")
  .developers([("Michael Murphy", "mmstick@system76.com")])
  .links([
      ("Website", "https://system76.com/cosmic"),
      ("Repository", "https://github.com/pop-os/libcosmic)"),
      ("Support", "https://github.com/pop-os/libcosmic/issues"),
  ])

@wiiznokes

[wiiznokes]

I like the new api!
Just some minor doc issues

[wiiznokes]

Could you add this method to support the fl! macro ?

pub fn link(mut self, name: impl Into<String>, link: &'a str) -> Self {
    self.links.insert(name.into(), link.to_string());
    self
}

Edit: or smtg like this

[edfloreshz]

I'm thinking of the best way to support localization with the existing set of methods.

[wiiznokes]

I'm thinking of the best way to support localization with the existing set of methods.

smtg like that could work:

pub fn links<const N: usize>(
    mut self,
    links: [(impl Into<String>, impl Into<String>); N],
) -> Self {
    self.links = links
        .into_iter()
        .map(|(k, v)| (k.into(), v.into()))
        .collect();
    self
}

[edfloreshz]

Decided to do this instead given that URLs don't really require localization, only labels.

pub fn links<T: Into<String>>(mut self, links: impl Into<Vec<(T, &'a str)>>) -> Self {
    let links: Vec<(T, &'a str)> = links.into();
    self.links = links
        .into_iter()
        .map(|(k, v)| (k.into(), v.to_string()))
        .collect();
    self
}

@wiiznokes Let me know if this works for you.

[wiiznokes]

Yeah, it works. The Vec also make probably more sense.

I noticed that the scrollbar is a bit off, but idk if it's related or not. Maybe we need to adjust the padding.

[edfloreshz]

Yeah we could give it some right padding

[edfloreshz]

@wiiznokes Please confirm if I can ask Michael to review.

[git-f0x]

Is there a reason that the scrollable was removed from the drawer? Since now it isn't possible to make the scrollbar be at the right edge (to match designs), and that spacing that's required to make the scrollbar not cover content makes things off-center.
Though I guess the container padding can be removed in the widget, and then that padding could be applied in every app separately, and then a scrollable applied. But that seems a bit cumbersome.

[wiiznokes]

@wiiznokes Please confirm if I can ask Michael to review.

Yeah, go ahead

[edfloreshz]

@mmstick This should be ready for review now.

[git-f0x]

If #698 gets merged, the scrollbar spacing can be removed, but .padding([0, space_l, space_l, space_l]) should be applied to the column (before the scrollable).