New, more modern KivaFont trait #929

corranwebster · 2022-04-08T16:59:21Z

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

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

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

mdickinson · 2022-04-21T14:56:08Z

I'll wait on review until #924 and #928 have been merged into main (and from there into this branch).

jwiggins

Some preliminary comments

enable/examples/demo/enable/editors/font_editor.py

enable/tests/trait_defs/test_kiva_font_trait.py

enable/trait_defs/api.py

enable/trait_defs/kiva_font_trait.py

jwiggins · 2022-04-22T08:08:46Z

kiva/trait_defs/kiva_font_trait.py

@@ -10,181 +10,7 @@
 """ Trait definition for a wxPython-based Kiva font.
 """

-import logging
+from enable.trait_defs.kiva_font_trait import KivaFont as _KivaFont


Are you sure we want to import from enable here?

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.

corranwebster · 2022-04-26T16:48:00Z

@mdickinson if you have time to review, this would be great.

mdickinson · 2022-04-26T16:50:22Z

Will review first thing tomorrow. Please could you fix the conflicts first?

corranwebster · 2022-04-26T17:00:31Z

Please could you fix the conflicts first?

Hopefully done - I'm at the point today where I am messing things up.

corranwebster · 2022-04-27T08:33:45Z

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 __init__.py)

mdickinson

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.

mdickinson · 2022-04-27T07:12:52Z

docs/source/enable/traits.rst

@@ -18,7 +18,7 @@ the form of an HTML color name ("blue" or "#0000FF").

 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`.


Nits:

Rewrap to avoid long lines? The rest of the file seems to be limited to width 80.

The statement seems misleading: font_trait is an instance of KivaFont, rather than a synonym for it. Worth rephrasing?

enable/examples/demo/enable/editors/font_editor.py

enable/label.py

enable/tests/trait_defs/test_kiva_font_trait.py

kiva/fonttools/font.py

kiva/trait_defs/tests/test_kiva_font_trait.py

kiva/fonttools/font.py

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>

corranwebster · 2022-04-27T12:56:48Z

I think that's everything addressed; assuming tests pass, this is ready for re-review.

enable/tests/trait_defs/test_kiva_font_trait.py

enable/examples/demo/enable/editors/font_editor.py

mdickinson · 2022-04-28T07:40:07Z

Aargh; sorry. I was somehow seeing out-of-date code in GitHub. All looks good.

mdickinson

LGTM

corranwebster added 2 commits April 8, 2022 10:45

corranwebster requested a review from mdickinson April 20, 2022 16:06

jwiggins reviewed Apr 22, 2022

View reviewed changes

corranwebster mentioned this pull request Apr 26, 2022

Add a new font-editor that uses Enable to draw fonts #928

Merged

corranwebster added 3 commits April 26, 2022 17:54

Merge branch 'main' into enh/new-font-trait

bd05d4b

Fix long line.

3fc6f67

Save all files before finishing merge!

32a3b36

mdickinson reviewed Apr 27, 2022

View reviewed changes

Fix tests.

555419e

corranwebster commented Apr 27, 2022

View reviewed changes

kiva/fonttools/font.py Outdated Show resolved Hide resolved

corranwebster and others added 2 commits April 27, 2022 10:21

Apply suggestions from code review

cb2b1da

Co-authored-by: Mark Dickinson <mdickinson@enthought.com>

Handle color editor tests for null backend.

82ea9fd

corranwebster mentioned this pull request Apr 27, 2022

Remove font traits and editors from Kiva #934

Open

corranwebster added 2 commits April 27, 2022 13:17

Fixes from PR review.

01a4109

Some more small fixes coming from PR discussion.

ea32353

corranwebster requested a review from mdickinson April 27, 2022 12:56

mdickinson reviewed Apr 28, 2022

View reviewed changes

enable/tests/trait_defs/test_kiva_font_trait.py Show resolved Hide resolved

mdickinson reviewed Apr 28, 2022

View reviewed changes

enable/examples/demo/enable/editors/font_editor.py Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

mdickinson approved these changes Apr 28, 2022

View reviewed changes

corranwebster merged commit 74536e0 into main Apr 28, 2022

corranwebster deleted the enh/new-font-trait branch April 28, 2022 07:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New, more modern KivaFont trait #929

New, more modern KivaFont trait #929

corranwebster commented Apr 8, 2022

mdickinson commented Apr 21, 2022

jwiggins left a comment

jwiggins Apr 22, 2022

corranwebster Apr 26, 2022 •

edited

Loading

corranwebster commented Apr 26, 2022

mdickinson commented Apr 26, 2022 •

edited by corranwebster

Loading

corranwebster commented Apr 26, 2022

corranwebster commented Apr 27, 2022 •

edited

Loading

mdickinson left a comment

mdickinson Apr 27, 2022

corranwebster commented Apr 27, 2022

This comment was marked as outdated.

mdickinson commented Apr 28, 2022

mdickinson left a comment

New, more modern KivaFont trait #929

New, more modern KivaFont trait #929

Conversation

corranwebster commented Apr 8, 2022

mdickinson commented Apr 21, 2022

jwiggins left a comment

Choose a reason for hiding this comment

jwiggins Apr 22, 2022

Choose a reason for hiding this comment

corranwebster Apr 26, 2022 • edited Loading

Choose a reason for hiding this comment

corranwebster commented Apr 26, 2022

mdickinson commented Apr 26, 2022 • edited by corranwebster Loading

corranwebster commented Apr 26, 2022

corranwebster commented Apr 27, 2022 • edited Loading

mdickinson left a comment

Choose a reason for hiding this comment

mdickinson Apr 27, 2022

Choose a reason for hiding this comment

corranwebster commented Apr 27, 2022

This comment was marked as outdated.

mdickinson commented Apr 28, 2022

mdickinson left a comment

Choose a reason for hiding this comment

corranwebster Apr 26, 2022 •

edited

Loading

mdickinson commented Apr 26, 2022 •

edited by corranwebster

Loading

corranwebster commented Apr 27, 2022 •

edited

Loading