Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Possible to set a negative line width #135

Closed
fedarko opened this issue Feb 3, 2020 · 3 comments · Fixed by #275
Closed

Possible to set a negative line width #135

fedarko opened this issue Feb 3, 2020 · 3 comments · Fixed by #275
Assignees
@fedarko
Labels
bug
Milestone
First Beta Release

Comments

@fedarko
Copy link
Collaborator

fedarko commented Feb 3, 2020 •
edited
Loading

I assume this isn't intended (if it's actually a feature feel free to close this :) -- assuming it isn't, should be possible to add in min="1" or something to the corresponding <input> element.

Edit: should also add JS that validates that the new branch thickness is within a range of acceptable values, as a failsafe. Relates to #144.

Screenshot of a -10 width below (I admit it does look kind of cool, though) --

issue
@ElDeveloper ElDeveloper added this to the Beta Release milestone Jul 13, 2020
@ElDeveloper
Copy link
Member

Related to #214

@ElDeveloper
Copy link
Member

Would be good to centralize and unify the setting of line widths.

fedarko added a commit to fedarko/empress that referenced this issue Jul 14, 2020
@fedarko
BUG: set min="1" on line width <input>s biocore#135
7e87580 
Of course, still need to handle case where user is obstinate and
types in "-1" or whatever manually anyway...
@fedarko
Copy link
Collaborator Author

fedarko commented Jul 14, 2020

I think it might work well to handle #144 alongside these changes for the next release, also.

@fedarko fedarko mentioned this issue Jul 17, 2020
Open
fedarko added a commit to fedarko/empress that referenced this issue Jul 17, 2020
@fedarko
MNT: add parseAndValidateLineWidth() and tests
b774d0a 
Per work on biocore#135 / biocore#144.
fedarko added a commit to fedarko/empress that referenced this issue Jul 18, 2020
@fedarko
BUG: Make all l.w. inputs go thru parseAndValidate
86d4eaa 
Closes biocore#135! It would be nice to have some Empress / side-panel /
animator tests that verify things work properly, but i think this is
ok for now.

Also of note: now, the default line width is 0. This will be important
for biocore#144.
@fedarko fedarko mentioned this issue Jul 23, 2020
Merged
@ElDeveloper ElDeveloper mentioned this issue Jul 24, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fedarko fedarko

Labels
bug
Projects
None yet
Milestone
First Beta Release
Development

Successfully merging a pull request may close this issue.

Validate input line widths; scale widths based on number of leaves in the tree fedarko/empress
2 participants
@ElDeveloper @fedarko