toga icon indicating copy to clipboard operation
toga copied to clipboard

Updated Pack properties to use new Travertino dataclass-based API

Open HalfWhitt opened this issue 11 months ago • 5 comments

This has been tested against the up-to-date Travertino with https://github.com/beeware/travertino/pull/141, but can't be merged here until there's a new release of Travertino and Toga's dependency is updated.

PR Checklist:

  • [x] All new features have been tested
  • [ ] All new features have been documented
  • [x] I have read the CONTRIBUTING.md file
  • [x] I will abide by the code of conduct

HalfWhitt avatar Mar 05 '24 23:03 HalfWhitt

@HalfWhitt I don't know if you've got any plans for more Travertino changes; the only thing that comes to mind for me was the issue about py3.13 support that I flagged during the review of the data class PR.

Once you're done, let me know and I'll cut a release to clear the path for this PR.

freakboy3742 avatar Mar 06 '24 07:03 freakboy3742

@freakboy3742 Well... I have started converting the rest of the tests to pytest. But that shouldn't affect anything outward-facing. Up to you whether you'd rather wait till that's done or not.

HalfWhitt avatar Mar 06 '24 09:03 HalfWhitt

On second thought, I suppose this would be a good time to see about implementing list_property and/or composite_property, assuming those are still intended to happen. I only know about them from the commented-out sections. It looks like composite_property is supposed to function something like how the font parser already does, splitting a string into the relevant aliased properties, is that correct?

HalfWhitt avatar Mar 06 '24 12:03 HalfWhitt

On second thought, I suppose this would be a good time to see about implementing list_property and/or composite_property, assuming those are still intended to happen. I only know about them from the commented-out sections. It looks like composite_property is supposed to function something like how the font parser already does, splitting a string into the relevant aliased properties, is that correct?

That's correct. composite_property would be used by a font property, and list_property by font_family - with the obvious complication that font_family is then the first part of `font.

Those two definitions have been skipped because Pack has been able to survive without them - for the purposes of a "CSS-lite", it's fine to require the more explicit font syntax. They've only been stubbed out because they will be required for Colosseum.

I guess the question about whether to wait or not is up to you. They're not super-high priorities for me personally, because there's a totally acceptable workaround - but it they motivate you, then it's probably worth waiting so the next Travertino release is a really juicy one. And if someone (cough) were to use this as the start of getting Colosseum back on track... I also would not complain :-)

freakboy3742 avatar Mar 06 '24 21:03 freakboy3742

Alright yeah, hold off for now, I think I'll try my hand at them. And no promises about Colosseum, but I had in fact been thinking of such a thing...

HalfWhitt avatar Mar 07 '24 02:03 HalfWhitt