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

auto completion can be toggled on and off #127

Merged
merged 4 commits into from
Aug 7, 2020
Merged

Conversation

frcroth
Copy link
Contributor

@frcroth frcroth commented Jun 28, 2020

Should fix #60

@frcroth frcroth requested review from konradh and sirkrypt0 July 1, 2020 08:55
@frcroth frcroth linked an issue Jul 1, 2020 that may be closed by this pull request
3 tasks
@frcroth frcroth added this to the Fifth Sprint milestone Jul 1, 2020
@sirkrypt0
Copy link
Contributor

I assume there is no way to test this?

@frcroth
Copy link
Contributor Author

frcroth commented Jul 2, 2020

I assume there is no way to test this?

@sirkrypt0 Maybe you could try to type and then check number of morphs / number of text suggestion morphs while typing?

@LeonMatthes
Copy link
Contributor

If you want to test the Autocompletion, make sure it's installed first.
You can add it as a dependency of your test package in your Baseline (repo is here ).

The API of the Autocompletion is mostly not guaranteed, but it still doesn't change very often.

You can access the Autocompletions controller using Model>>#completionController and then use ECController>>#isMenuOpen to check whether the autocompletion is opened.

Copy link
Contributor

@konradh konradh left a comment

Choose a reason for hiding this comment

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

I'm not really sure why this is necessary as RichTextEditorModel>>#initializeCompletionController is now exactly the same as Model>>#initializeCompletionController, which we inherit anyway.
It should make absolutely no difference.
Enabling and disabling autocompletion already worked for me before.

@frcroth
Copy link
Contributor Author

frcroth commented Jul 3, 2020

I'm not really sure why this is necessary as RichTextEditorModel>>#initializeCompletionController is now exactly the same as Model>>#initializeCompletionController, which we inherit anyway.
It should make absolutely no difference.
Enabling and disabling autocompletion already worked for me before.

It doesn't seem to work for anyone else usually, but on this branch it does work for me

@frcroth frcroth self-assigned this Jul 3, 2020
@sirkrypt0
Copy link
Contributor

If you want to test the Autocompletion, make sure it's installed first.
You can add it as a dependency of your test package in your Baseline (repo is here ).

The API of the Autocompletion is mostly not guaranteed, but it still doesn't change very often.

You can access the Autocompletions controller using Model>>#completionController and then use ECController>>#isMenuOpen to check whether the autocompletion is opened.

I think we should do that to really make sure it can be toggled off and on reliably.

@sirkrypt0
Copy link
Contributor

I'm not really sure why this is necessary as RichTextEditorModel>>#initializeCompletionController is now exactly the same as Model>>#initializeCompletionController, which we inherit anyway.
It should make absolutely no difference.
Enabling and disabling autocompletion already worked for me before.

It doesn't seem to work for anyone else usually, but on this branch it does work for me

I agree, it works here

utilities
enableAutocompletion

autocompletionPreference := RichTextEditorModel autocompletion.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why no accessors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not necessary

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we did settle on always using them

…ss/instance/enableAutocompletion.st

Co-authored-by: sirkrypt0 <22522058+sirkrypt0@users.noreply.github.com>
@frcroth frcroth modified the milestones: Fifth Sprint, Sixth Milestone Jul 13, 2020
@frcroth
Copy link
Contributor Author

frcroth commented Jul 13, 2020

todo: Test should really test for autocompletion (acceptance test)

@frcroth frcroth modified the milestones: Sixth Sprint, Final Sprint Aug 3, 2020
@sirkrypt0
Copy link
Contributor

maybe consider adding these changes on a new branch to avoid merge conflicts

@frcroth frcroth changed the base branch from dev to autocompletion-test August 7, 2020 14:07
@frcroth frcroth merged commit 84e2b4a into autocompletion-test Aug 7, 2020
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.

Disabling auto completion suggestions
5 participants