-
Notifications
You must be signed in to change notification settings - Fork 221
fix: correctly type the size property #289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
0a1a65f to
f3f84c7
Compare
| y: number | ||
| } | ||
|
|
||
| export type IAreaSize = number | '*' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weird to prefix a type (not an interface) with the capital I prefix that means Interface :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point @lf-novelt - I'm not a fan of the I for interfaces either, it's more a matter of keeping consistency.
|
Thanks for the PR. |
|
Has this been merged yet? Still having the issue; using the $any('*') workaround for now |
Sorry, little time for maintenance currently. I will see if I can cut a release and include this change. |
|
Can you merge this modification? |
|
Is there a plan to release this PR? |
|
@beeman The PR seems good... Any reason to wait with it? |
|
For the people who are waiting this PR. This PR is still waiting for some reasons. For example: <as-split
[direction]="'horizontal'"
[unit]="'pixel'"
>
<as-split-area
[size]="280"
[maxSize]="280">
A
</as-split-area>
<as-split-area
[size]="$any('*')">
B
</as-split-area>
</as-split> |
In situations where we hide the text, intending just for the audio
player to be used, the patch makes the text+audio area have a maximum
size of the height needed by the audio player (109 px), to just show
the audio player. The audio area can still be made smaller to move it
up out of the way.
In the case where we do not hide the text, and whether or not there is
audio, still we show the question area as a smaller area near the
bottom of the screen, with the Scripture text area using most of the
available space. Though I made the question area slightly bigger than
it was before so the "Add answer" button has a greater chance of being
fully visible (170 px).
Using `$any('*')` workaround until this PR is merged:
angular-split/angular-split#289
After that, just `'*'` can be used.
In situations where we hide the text, intending just for the audio
player to be used, the patch makes the text+audio area have a maximum
size of the height needed by the audio player (109 px), to just show
the audio player. The audio area can still be made smaller to move it
up out of the way.
In the case where we do not hide the text, and whether or not there is
audio, still we show the question area as a smaller area near the
bottom of the screen, with the Scripture text area using most of the
available space. Though I made the question area slightly bigger than
it was before so the "Add answer" button has a greater chance of being
fully visible (170 px).
Using `$any('*')` workaround until this PR is merged:
angular-split/angular-split#289
After that, just `'*'` can be used.
In situations where we hide the text, intending just for the audio
player to be used, the patch makes the text+audio area have a maximum
size of the height needed by the audio player (109 px), to just show
the audio player. The audio area can still be made smaller to move it
up out of the way.
In the case where we do not hide the text, and whether or not there is
audio, still we show the question area as a smaller area near the
bottom of the screen, with the Scripture text area using most of the
available space. Though I made the question area slightly bigger than
it was before so the "Add answer" button has a greater chance of being
fully visible (170 px).
Using `$any('*')` workaround until this PR is merged:
angular-split/angular-split#289
After that, just `'*'` can be used.
Fix #220