New, more modern KivaFont trait#929
Conversation
This font editor is laregly toolkit independent - it embeds a Window with a Label component to display the font and the simple editor uses the Pyface font dialog to get new font values. Some functionality is broken out into experimental base classes for building editors with components.
This creates a KivaFont TraitType, which is: - in enable, rather than Kiva, meaning Kiva can potentially not need traits - has a better font parser that supports more weight types - the font parser can be swapped out for something better easily - uses the new KivaFontEditor, which is available for Qt - converts Pyface Fonts to Kiva Fonts
| """ | ||
|
|
||
| import logging | ||
| from enable.trait_defs.kiva_font_trait import KivaFont as _KivaFont |
There was a problem hiding this comment.
Are you sure we want to import from enable here?
There was a problem hiding this comment.
Yes, I think so. It's preliminary to getting rid of kiva.trait_defs.
As long as we don't have circularity of imports it should be fine.
|
@mdickinson if you have time to review, this would be great. |
|
Will review first thing tomorrow. Please could you fix the conflicts first? |
Hopefully done - I'm at the point today where I am messing things up. |
|
Looks like some imports got mangled/moved in the merge. Not entirely sure how the tests are passing right now. Edit: because tests for trait defs weren't in a package (no |
mdickinson
left a comment
mdickinson
left a comment
There was a problem hiding this comment.
Done with first-pass review. Lots of comments, mostly at nitpick level.
At some point, it would be great to get flake8-style checks up and running on this repository; it's more efficient at catching things like NameErrors than humans are.
| font_trait | ||
| ---------- | ||
| :class:`~.font_trait` is a synonym for :class:`kiva.trait_defs.api.KivaFont`. | ||
| :class:`~.font_trait` is a synonym for :class:`enable.trait_defs.kiva_font_trait.KivaFont`. |
There was a problem hiding this comment.
Nits:
- Rewrap to avoid long lines? The rest of the file seems to be limited to width 80.
- The statement seems misleading:
font_traitis an instance ofKivaFont, rather than a synonym for it. Worth rephrasing?
Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
|
I think that's everything addressed; assuming tests pass, this is ready for re-review. |
This comment was marked as outdated.
This comment was marked as outdated.
|
Aargh; sorry. I was somehow seeing out-of-date code in GitHub. All looks good. |
3 participants
This creates a KivaFont TraitType, which is:
This requires #928 to be merged before it is.
In conjunction with #924 this closes #417
It also closes #922 and closes #896 and closes #471