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

Word count goal progress bar broken in develop. #652

Closed
vithiri opened this issue Sep 20, 2019 · 4 comments
Closed

Word count goal progress bar broken in develop. #652

vithiri opened this issue Sep 20, 2019 · 4 comments
Milestone
0.10.0

Comments

@vithiri
Copy link

vithiri commented Sep 20, 2019

The word count progress bar currently seems to be broken in develop while working fine in master.

Steps to reproduce:

Set a word goal for a text and start typing

Expected outcome:

Get a blue growing progress bar in the lower right hand corner of the editor

Actual outcome:

The area where the bar is supposed to be is displaying next to the word count, but the bar itself never fills up
@worstje
Copy link
Contributor

worstje commented Sep 20, 2019

Thank you.

I have already traced this back to the fix for issue #561 which was fixed in PR #609, and am currently looking into a proper way to fix this.

@gedakc
Copy link
Collaborator

gedakc commented Sep 20, 2019

Good catch @vithiri. We'll want to fix this before releasing 0.10.0.

Thanks @worstje for looking into this. It might be an automatic type conversion thingy defaulting to either an integer or a real number. For example: 100 / 500 = 0.2 real = 0 integer

@gedakc gedakc added this to the 0.10.0 milestone Sep 20, 2019
worstje added a commit to worstje/manuskript that referenced this issue Sep 22, 2019
@worstje
Restore progress bar functionality
2582b8b 
A previous fix (5f9ea3) inadvertently broke the progress bar by
converting to the wrong data type. (See issue olivierkes#561 / PR olivierkes#609).

While checking the code I realized the problem occurred primarily
because we weren't checking the validity of the values closer to the
source. Doing so alleviates the need to check elsewhere.

In the hope of inspiring a more systematic approach, a new uiParse()
utility function has been added to curb the further growth of toXxx()
functions that exist solely to validate user input.

There is no doubt room for improvement, both on the end of the new
uiParse() function as well as the spot where it is used. Ideally, the
data that comes out of the model should already be 'safe', but since
this is a bugfix for a bugfix I want to keep waves to a minimum.

This commit fixes issue olivierkes#652.
@worstje worstje mentioned this issue Sep 22, 2019
Merged
@worstje
Copy link
Contributor

worstje commented Sep 22, 2019

@gedakc You were completely right. :)

I took the fix a little bit further in the interest of encouraging a more systematic approach to user input validation in the future. I hope that's not a problem. 😄

gedakc pushed a commit that referenced this issue Sep 23, 2019
@worstje @gedakc
Restore progress bar functionality
3aa9cad 
A previous fix (5f9ea3) inadvertently broke the progress bar by
converting to the wrong data type. (See issue #561 / PR #609).

While checking the code I realized the problem occurred primarily
because we weren't checking the validity of the values closer to the
source. Doing so alleviates the need to check elsewhere.

In the hope of inspiring a more systematic approach, a new uiParse()
utility function has been added to curb the further growth of toXxx()
functions that exist solely to validate user input.

There is no doubt room for improvement, both on the end of the new
uiParse() function as well as the spot where it is used. Ideally, the
data that comes out of the model should already be 'safe', but since
this is a bugfix for a bugfix I want to keep waves to a minimum.

This commit fixes issue #652.
@gedakc
Copy link
Collaborator

gedakc commented Sep 23, 2019

Closing this issue as it is resolved with the merge of PR #654.

@gedakc gedakc closed this as completed Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.10.0
Development

No branches or pull requests
3 participants
@worstje @vithiri @gedakc