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.

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

Improvements to the calculator. #5626

Merged
merged 47 commits into from
Feb 3, 2020

Conversation

Wycklife
Copy link
Contributor

@Wycklife Wycklife commented Jan 7, 2020

Fixes #5623
This fixes the first 5 parts of the issue.
@africanmathsinitiative/developers This can be reviewed

@Wycklife
Copy link
Contributor Author

Wycklife commented Jan 7, 2020

@rdstern I have fixed number 1-5. I need more clarification on number 6 and 7 kindly.

@rdstern
Copy link
Collaborator

rdstern commented Jan 8, 2020

Hi @Wycklife
I reply first on the changes you have done to items 1 to 5. Then later to clarify 6 and 7.

  1. Make the dialogue a bit wider for this keyboard.
  2. Add the "stringr::" to the new keys as shown in item 3 in the list. I assume it is needed?
  3. Did you do item 4? I couldn't see the pattern = argument in those keys?

@Wycklife
Copy link
Contributor Author

Wycklife commented Jan 8, 2020

@rdstern Thanks for the reply, I did item 4. but I can as well have a check on them.

@rdstern
Copy link
Collaborator

rdstern commented Jan 8, 2020

Here is an indication of the complete keyboard(s) with the items 6 and 7:
(Not quite like this of course - I hope the other keyboards won't go so far down in the dialogue. But I hope the picture explains what could be useful. And this relates also to your proposed presentation in the Zambia meeting.

image

@Wycklife
Copy link
Contributor Author

@rdstern I have implemented the two keyboards. Could you kindly check whether the parameters passed by the keys are correct. I have changed the location of the base so as to allow full display of the string keyboard but I have realized it affects other keyboards i'e creating more space. I will ask @dannyparsons to help me solve the problem once he's done with the session.

@Wycklife
Copy link
Contributor Author

@rdstern I'm happy to make changes you'll suggest.

@Wycklife
Copy link
Contributor Author

@africanmathsinitiative/developers This is now ready for review.

@rdstern
Copy link
Collaborator

rdstern commented Jan 23, 2020

@Wycklife this is looking good. So just small suggestions.
a) The group box for Modifiers has an extra line that can be deleted (Compare with the others, Strings or Symbols where there is no extra line.)
b) The symbols keys would ideally be of similar size (as far as possible) so they line up vertically.
c) In the symbol keys you have sometimes added brackets - first 2 rows - and I am not sure they are needed. Can you try without. Then I assume the \ s symbol will have \s space where there wasn't room for \ s (space)?
d) In \ d and \ s there should be no space between \ and s - or \ and d.
e) In the [ ] and [^ ] leave the cursor within the brackets - after the ^. It is now after the brackets.
f) In the {n} and {m,n} keys the key should deliver { } and { , }, i.e. without the n or m and with the cursor just after the first bracket.
g) On the bottom row the + 0 or 1 key should just deliver the + and the same for the other 2 keys.

@Wycklife
Copy link
Contributor Author

@rdstern I have made the changes. Kindly have a look.

@maxwellfundi
Copy link
Contributor

@rdstern this seems to have been fixed from your comments above.

Cheers.

@maxwellfundi maxwellfundi merged commit 03b888e into IDEMSInternational:master Feb 3, 2020
@Wycklife Wycklife deleted the Keyboards branch February 4, 2020 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improvements to the calculator - mainly character keyboard
3 participants