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

Added Trend keyboard and general changes to layout for the Hypothesis Tests dialog #5821

Merged
merged 78 commits into from
Jul 6, 2020

Conversation

Wycklife
Copy link
Contributor

@Wycklife Wycklife commented Jun 5, 2020

Fixes #5816 and #5814

@africanmathsinitiative/developers This is ready for review

@rdstern
Copy link
Collaborator

rdstern commented Jun 5, 2020

@wyclife - this is looking good.
a) On the keys from the third row there is ::: (i.e. 3 colons) rather than just 2. In the bottom row it rises to ::::.
b) the tooltip for mk has Trend Test, rather than trend test. (All the other seem consistent - great.)
c) Put the Clear button before (and on the left, next to the Help), rather than underneath it.
d) Move the Try control slightly left, so it is aligned with the check boxes above and then it can also move upwards so it is next to the bottom row of the symbol keys. Move the other keys at the left so they all align and also so that the dialogue becomes less deep.
e) Finally something new that I need to check with @dannyparsons. In some commands we need to include the data frame name in the command. Currently this can't be done, i.e. you can't click and Add the data-frame name. If this is easy it would be useful to add - otherwise it should not holdup the merging, but can become a new issue.

@Wycklife
Copy link
Contributor Author

Wycklife commented Jun 8, 2020

@rdstern I have fixed the first 4 points above. @africanmathsinitiative/developers Could someone have a look at these changes.

@rdstern
Copy link
Collaborator

rdstern commented Jun 9, 2020

@Wycklife you have fixed the first 3 - here is the 4th

d) Move the Try control slightly left, so it is aligned with the check boxes above (if aligned now, then move them all a bit left) and then it can also move upwards so it is next to the bottom row of the symbol keys. Move the other keys at the left so they all align and also so that the dialogue becomes less deep.

Ignore the final point (e) above for now. I have discussed with @dannyparsons and will make a separate issue.

@Wycklife
Copy link
Contributor Author

@rdstern Done. Kindly check a gain.

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Wycklife , that is a bit better but not quite right yet. We have just started adding @lilyclements back into the team and re-organising dialogues is one of her strengths. So I suggest it remains as your pull request, but she will do some edits of the layout. She is likely to have more ideas than those I gave you! Let's see.

@Wycklife
Copy link
Contributor Author

@rdstern That's fine.

@lilyclements
Copy link
Contributor

@Wycklife would you mind updating this branch from the master copy?

@Wycklife
Copy link
Contributor Author

@rdstern I have merged the changes made by @lilyclements into this branch. Could you please check if the designer is now okay.

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am happy to approve. I am commenting so @lloyddewit can check and hopefully approve.

@lloyddewit
Copy link
Contributor

@maxwellfundi I reviewed, it looks good and I''m happy to approve. Please merge, thanks

@lloyddewit
Copy link
Contributor

@rdstern You already approved this. Please could you approve again so that we can merge? thanks

1 similar comment
@lloyddewit
Copy link
Contributor

@rdstern You already approved this. Please could you approve again so that we can merge? thanks

Copy link
Collaborator

@rdstern rdstern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to approve again

@lloyddewit lloyddewit merged commit 5108761 into IDEMSInternational:master Jul 6, 2020
@Wycklife Wycklife deleted the TrendKeyboard branch September 1, 2020 10:41
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.

Add trend keyboard to Model > Fit Model > Hypothesis Testing Keyboard
4 participants