The UX of resizing small images. #9173
Conversation
magda-chrzescian
force-pushed
the
i/9166-resizing-small-images
branch
from
701cf43 to
948d9cc
Compare
| /** | ||
| * The position of the view defined based on the host size and active handle position. | ||
| * | ||
| * @private |
There was a problem hiding this comment.
There was a problem hiding this comment.
Also @readonly, no one will ever write to this property.
There was a problem hiding this comment.
It is not used in tests anymore, so I left it @Private.
Asserting viewPosition makes little sense here IMO because it is an internal component state for binding purposes only. The only thing that matters for this component is the class.
| bind.to( 'viewPosition', value => value ? `ck-orientation-${ value }` : '' ) | ||
| ], | ||
| style: { | ||
| display: bind.if( 'isVisible', 'none', visible => !visible ) |
There was a problem hiding this comment.
Missing docs for #isVisible. Also, #_isVisible should also be protected.
There was a problem hiding this comment.
It is not used in tests or outside the class actually, so I made it @Private.
| } | ||
| }, | ||
| children: [ { | ||
| text: bind.to( 'label' ) |
There was a problem hiding this comment.
Missing #label docs. This also looks like an internal state for template binding purposes, so let's make it @Protected and #_label.
There was a problem hiding this comment.
It is not used in tests or outside the class actually, so I made it @Private.
| import SizeView from '../../src/widgetresize/sizeview'; | ||
| import ResizerState from '../../src/widgetresize/resizerstate'; | ||
|
|
||
| describe( 'SizeView', () => { |
There was a problem hiding this comment.
If we're testing this view properly now, then some essential tests are missing:
- template structure assertion,
style.displaytests,- children text (label) tests.
Suggested merge commit message (convention)
Fix (image): An image should never overflow the widget boundaries while changing its size. Closes #9166.
Fix (image): The size label should be displayed above the image if it doesn't fit inside. See #9166.
Fix (image): An Image caption placeholder text should not wrap. Closes #9162.
AFAIR the
SizeViewclass was not tested directly at all. I exported it (it is declared as public in the API docs) and wrote the tests covering only the purpose of this issue.Requires #9043.