rajdeep / proton

Purely native and extensible rich text editor for iOS and macOS Catalyst apps
Other
1.26k stars 81 forks source link

New main #240

Closed dyljqq closed 10 months ago

rajdeep commented 10 months ago

@dyljqq it seems that this is a duplicate of #231 and there are a couple more that you have added i.e. #238 and #239 . Please do not open duplicate PRs. Also, it seems that none of the concerns raised in #231 have been addressed in this.

When raising a PR, please ensure that the code is building and all the tests are passing. Also, the changes in your PR are not aligned with the platform philosophy being followed in Proton. When making a change, please try to make it at a level that it does not open up the code to accidental misuse. Many of the classes, vars and functions are intentionally kept private/internal and when providing anything in public API, we must think about how the consumers of this framework will use it. Since Proton is a generalized framework, the API should be designed in a way that it only works for some specific case.

As I mentioned earlier as well, I do have plans to add a checkbox list in Proton in future. As it seems that these changes are something that is working for you, you may continue to use your fork until the checkbox list support is added in Proton. Alternatively, you can also create Attachment with a checkbox and EditorView and use that as a checkbox list item in your code. You can also handle indentation using Tab and Shift+Tab by intercepting the EditorKey. Please refer to ReadMe for an example of key handling. Your code may look something like this:

extension PanelAttachment: PanelViewDelegate {

func panel(_ panel: PanelView, didReceiveKey key: EditorKey, at range: NSRange, handled: inout Bool) {
    if key == .enter,
     let rangeInContainer = self.rangeInContainer() {
        // get reference of containerEditorView
        // get contentLineInRange from rangeInContainer
        // change pagaraphStyle to add desired indentation to paragraph in contentLineInRange
        handled = true
        }
    }
}    
rajdeep commented 10 months ago

If you would like to merge your changes, please review your code based on the guidelines provided in my earlier comment and ensure that the concerns raised in #231 are addressed appropriately. Please note that not all the code issues have been highlighted in #231 but I have ensured to highlight the type of concerns that I would like to see addressed.

Once again, I appreciate the effort you have put in this. Thank you.