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

Feature: Toolbar menu enhancement #8

Closed
3 tasks done
LuchoTurtle opened this issue Sep 25, 2023 · 7 comments · Fixed by #9
Closed
3 tasks done

Feature: Toolbar menu enhancement #8

LuchoTurtle opened this issue Sep 25, 2023 · 7 comments · Fixed by #9
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality flutter Flutter related issues priority-1 Highest priority issue. This is costing us money every minute that passes.

Comments

@LuchoTurtle
Copy link
Member

LuchoTurtle commented Sep 25, 2023

Having had a look over dwyl/app#275, since we now have a basic editor that works on both web and mobile, the next step would be to enhance it, according to the specifications in dwyl/app#275 (comment).

Here's my plan:

  • add emojis custom embed button.
  • add header buttons (up to h3).
  • add embeddable links.

image

@LuchoTurtle LuchoTurtle added documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality flutter Flutter related issues priority-1 Highest priority issue. This is costing us money every minute that passes. labels Sep 25, 2023
@LuchoTurtle LuchoTurtle self-assigned this Sep 25, 2023
@LuchoTurtle LuchoTurtle moved this to 🏗 In progress in dwyl app kanban Sep 25, 2023
@LuchoTurtle LuchoTurtle removed the status in dwyl app kanban Sep 25, 2023
@LuchoTurtle LuchoTurtle moved this to 🏗 In progress in dwyl app kanban Sep 25, 2023
@LuchoTurtle
Copy link
Member Author

LuchoTurtle commented Sep 25, 2023

Probably due to XCode/iOS 18 updates that I inclusively experienced last week (having to re-download the emulators again because of this), I'm having trouble running the app on an iPhone. I get the following.

Could not build the precompiled application for the device.
Error (Xcode): type argument 'nw_proxy_config_t' (aka 'struct nw_proxy_config *') is neither an Objective-C object nor a block type
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS17.0.sdk/System/Library/Frameworks/WebKit.framework/Headers/WKWebsiteDataStore.h:119:46

Parse Issue (Xcode): Could not build module 'WebKit'
/Users/lucho/Documents/dwyl/flutter-wysiwyg-editor-tutorial/build/ios/Debug-iphoneos/flutter_inappwebview/flutter_inappwebview.framework/Headers/flutter_inappwebview-Swift.h:285:8


Error launching application on iPhone.

Going to try pichillilorenzo/flutter_inappwebview#1735 (comment).

@nelsonic
Copy link
Member

Cool. Keep us posted. Thanks. 👌

@LuchoTurtle
Copy link
Member Author

LuchoTurtle commented Sep 25, 2023

Created #9, as these are still in-progress. Haven't yet committed the implementation of the emoji custom button because I want to do a slight refactor to separate concerns to make the code more readable. I'm documenting as I go.

Had a bit of trouble with the custom button, mostly because I was using a separate package for this emoji_picker_flutter to show the emoji picker and, after choosing, changing the controller Delta document afterwards. The behaviour was mighty weird because I kept losing focus of the document and the emoji wasn't being properly inserted/rendered. To fix this, I had to add a focusNode and conditionally unfocus and focus the editor controller so text would properly be inserted.

Inserting the emoji was another pain in my shoes. Since I wasn't using the native keyboard emojis (there is no way to invoke the native's keyboard emoji selection on all platforms, otherwise I would have used https://pub.dev/packages/keyboard_emoji_picker), I had to hijack the controller and properly insert the string in the document. This wasn't working, causing the app to crash. I had to dabble with the controller.selection to get the pointer and properly increment the base offset so the text emoji would be inserted normally (or else, emojis would be added always from the pointer, not from the last emoji that was inserted).

@nelsonic
Copy link
Member

Thanks for the update. As you know, I'm very keen to get the existing functionality we already have into the App. 📱
So please make an assessment what you need to do in the next couple of days to make that happen. 🙏

@LuchoTurtle
Copy link
Member Author

LuchoTurtle commented Sep 26, 2023

I’ve added all of the three requirements. I’ve spent a chunk of time trying to get testing to work so the emojis would be covered but, after spending much of my time actually getting the tests to work (the package https://github.com/Fintasys/emoji_picker_flutter/tree/master/test doesn’t do any widget testing at all, therefore there’s nothing there I can work by), I finally got it to work.

However, what left me stumped is that it’s not being correctly covered (for some odd reason). I’ve inclusively made a separate integration test with the same code that is used in the widget unit test to see the test flow in a real device and it’s properly working.

23-09-26-16-03-51.mp4

So I don’t know how to make it “get covered”. I’ve added the test (and the integration test setup so people could run it on real devices) and committed it to this PR. But I just wanted to leave this note here.

LuchoTurtle added a commit that referenced this issue Sep 26, 2023
LuchoTurtle added a commit that referenced this issue Sep 26, 2023
@LuchoTurtle LuchoTurtle moved this from 🏗 In progress to ⏳Awaiting Review in dwyl app kanban Sep 26, 2023
LuchoTurtle added a commit that referenced this issue Sep 26, 2023
@nelsonic
Copy link
Member

@LuchoTurtle thanks for the note on coverage. 🤔
Is this question covered (no pun intended) by a question/answer on StackOverflow? ❓
If not, please open that question when you are next at your desk.

@github-project-automation github-project-automation bot moved this from ⏳Awaiting Review to ✅ Done in dwyl app kanban Sep 27, 2023
@LuchoTurtle
Copy link
Member Author

Opened an issue in Fintasys/emoji_picker_flutter#156, as this only seems to occur in the onEmojiInserted callback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or enhancement of existing functionality flutter Flutter related issues priority-1 Highest priority issue. This is costing us money every minute that passes.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants