-
Notifications
You must be signed in to change notification settings - Fork 39
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
fix: 🍰 Suggestion List Filter #4296
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ashleysylvialee !
That looks really nice and well done !!! 🚀💫
An easy and good solution 🍀
Thanks for your contribution! 🙏🏼
I like to approve 👍🏼
… if the tests will run, what has nothing to do with your changes @ashleysylvialee .
Just in case you are interested on:
Would be cool if you could add a test for this in webapp/components/Editor/Editor.spec.js
.
I see you have had this already in mind … ☝🏼
If you need any help for this, please let us know.
@Mogge may you have a look at failing tests in the backend?
I have a little additional functionally suggestion, if you are interested: I would await that if I type a fitting slug, see ☝🏼, and hit the space key that the selected menu item will be chosen. There is a similar example for hashtags already in the code @ashleysylvialee : Ocelot-Social/webapp/components/Editor/Editor.vue Lines 184 to 188 in b1dec55
Just in case you are interested … 😍 |
I fixed the failing tests. Feel free to merge, although I like the suggestion @Tirokk brought up |
Done! The tests I wrote are passing as well. 🥳 Anything else you'd like me to add here? 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @ashleysylvialee .
Really cool job you’ve done!!!
Our users will be happy about your work and effort you spend. 🚀🚀💫💫
Great to have you in the project.
So I just fixed the linting: yarn lint --fix
And I approve.
Please merge!
PS: In case you change the locales they should be checked as well by : yarn locales --fix
🍰 Pullrequest
Issues
Todo