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

Github: Misc Issue cleanup #1500

Closed
innocuo opened this issue May 7, 2019 · 19 comments
Closed

Github: Misc Issue cleanup #1500

innocuo opened this issue May 7, 2019 · 19 comments
Labels

Comments

@innocuo
Copy link
Collaborator

@innocuo innocuo commented May 7, 2019

Randomly looking at issues, I ended up gathering a list of multiple ones that can be closed, labeled and merged. I prefer to post one single list rather than flooding the site with a bunch of posts.

I hope you find this useful:

Fixed issues, should be closed:

  • #222 There's a merged PR for it. Describes design issues from an older version.
  • #862 Selected note in search remains selected after clearing search.
  • #1103 pressing tab on title field no longer focuses on editor buttons
  • #1283, no need for discussion about missing translations. At most, ask for help in Contributing.md and the forums.
  • #1365 material design, not implemented, but person that was working on PR moved on. There are multiple issues and discussions on design, the abandoned ones shouldn't stay open.

Support requests that should be closed:

  • #1414 Author labeled it as Solved.
  • #1184 User hasn't replied in 2 months. As above, this is more a support request than a bug.

Deserve a "tag-ui-issue" label

I suggest you add labels to group related issues, once there's a pattern (when there are 3 or more that aren't duplicates, but closely related):

  • #864 is about tag renaming and duplication (and capitalization)
  • #1221 about duplicated tags
  • #1370 tag issues in terminal, including sync issues and tag duplication
  • #994 tag issues in terminal and android. Might be a duplicate of #1370

Used label: tags

Deserve a "tag-in-editor" label?

or merge discussions into one?

  • #1498 #1399 #1407 are all about making it easier to add tags from the editor.

Used labels: tags, editor

Duplicates

  • #716 vs #1451 about adding next/prev navigation (like browsers)
    #716 also includes a request that is already done

  • #1210 vs #1497 a weird character appears when entering Japanese or Korean text. This might be unfixable as it might be an Ace issue.
    #1351 should be closed. It's the same as above + some emoji issue also reported in #1410

Ace editor

Ace issues can't be fixed by Joplin. Joplin also depends on brace, and brace seems abandoned, so any editor improvements might not be doable.

Please add an "editor-issue" or "can't fix" to group them. And/or close them and add a "known editor issues" in the documentation or a new issue gathering all of them?

  • #971 Unicode chars with variable width mess up cursor position
  • #524 same as above, but for unicode emoji. This worked for me though.
  • #275 Spell check can't be implemented because Ace doesn't support spellcheck.
  • #1210 and #1497 seem like Ace, but might needs investigation to confirm. (needs an "editor-issue label"?)
  • #498 valid. It needs discussion, research (needs an "editor-issue label"?)
  • #1322 monospace fonts aren't supported by the editor. (also asks for a making preview panel editable, related to question below)

Are there plans to move to a different editor?

There are multiple issues here and in the forum. There should be a pinned forum post or issue, so that you don't get the same request over and over.

These should merge into one discussion or create a "wysiwyg-in-editor" label:

@tessus

This comment has been minimized.

Copy link
Collaborator

@tessus tessus commented May 7, 2019

This is definitely useful. Thank you. I meant to something similar a while back, but never got around to it.
(btw, #1413 has been closed for almost 3 weeks now)

Not quite sure how to handle the editor issues. Maybe @laurent22 has an idea.

I wanted to create a bunch of labels a while back. I was also thinking about using nested labels. Something like component:editor, component:tags, ... In that case all labels belonging to the group component could use the same color. The issues are getting more and more speciffic thus labelling them properly might be a good idea. This is a very good topic to discuss.
@laurent22 do you want to discuss this here or on the forum?

@tessus tessus pinned this issue May 7, 2019
@innocuo

This comment has been minimized.

Copy link
Collaborator Author

@innocuo innocuo commented May 7, 2019

(btw, #1413 has been closed for almost 3 weeks now)

I meant #1414 , I updated the top post accordingly.

@laurent22

This comment has been minimized.

Copy link
Owner

@laurent22 laurent22 commented May 7, 2019

@laurent22 do you want to discuss this here or on the forum?

Here it's fine

@innocuo

This comment has been minimized.

Copy link
Collaborator Author

@innocuo innocuo commented May 8, 2019

i removed #1186 from the list above, it turns out it's a real bug

@tessus

This comment has been minimized.

Copy link
Collaborator

@tessus tessus commented May 8, 2019

@innocuo #862 looks like a valid issue. I won't close this one.

@tessus

This comment has been minimized.

Copy link
Collaborator

@tessus tessus commented May 8, 2019

@laurent22 can you have a look at the duplicates? I'm not entirely sure how to handle those. It's true, they are pretty much the same, but I think you will have to make that determination.

@tessus

This comment has been minimized.

Copy link
Collaborator

@tessus tessus commented May 8, 2019

@laurent22 I have made a preliminary list to get the ball rolling. The component list is by far not complete and I kindly ask you to come up with additional ones.

Having labels organized this way helps a lot to group issues and plan further development.

Let me know what you think about the following labels:

platform:Linux
platform:macOS
platform:Windows
platform:Android
platform:iOS

and

component:editor
component:synchronizer or component:sync
component:toolbar
component:preview
component:tags
component:notebook
component:note
component:export
component:import
component:settings
component:highlighter

I suggest we use one color for platform: and another color for component:. Any suggestions?

@tessus

This comment has been minimized.

Copy link
Collaborator

@tessus tessus commented May 10, 2019

I'm not quite sure what to do with clipper and API. Maybe move clipper to component -> component:clipper and create 2 new labels for browser specific issues:

clipper:firefox
clipper:Chrome

I think API could also be moved to component:.

Btw, I have always used question for a ticket that was a usability question or something that didn't look like a bug report or feature request. It seems that you use it for a W4F (waiting for feedback from the OP), is that correct?

@laurent22

This comment has been minimized.

Copy link
Owner

@laurent22 laurent22 commented May 15, 2019

Thanks for looking into this @innocuo. I didn't have time to look into this so far, but your list makes sense. I've sent you a message on the forum, but basically I'm fine with most of your suggestions.

Regarding Brace, I didn't realise it was actually abandoned. One thing I've realised with this project is that it's better to avoid React wrappers like Brace. It's easy enough to do one's own wrapper and then we can update the wrapped library and configure it easily. When I switch to Monaco, that's the approach I'll follow - there'll just be a thin custom wrapper over the Monaco lib. For these issues, I agree we can tag them with something like "brace-on-hold" as they won't be fixed now, but we should revisit them once the editor has been updated.

For WYSIWYG, currently there's no plan to support it. If a good open source wysiwyg JS editor ever exists we can reconsider, but for now every time I review the existing options, I can't find any that would allow the customisations that we would need.

For the tags, I'm not too keen on the namespaces as there are not enough reasons to disambiguate. For example we know what the "linux" tag refers to, so "platform:linux" is not necessary. What I mean is that there's not enough tags and the project is not that big to justify the namespaces. I don't mind if we add some more tags to sort out the issues though.

@laurent22

This comment has been minimized.

Copy link
Owner

@laurent22 laurent22 commented May 15, 2019

Actually for the text editor I got confused - we use react-ace, which still appears to be active. Brace is only used for the themes as far as I can see.

@innocuo

This comment has been minimized.

Copy link
Collaborator Author

@innocuo innocuo commented May 16, 2019

Actually for the text editor I got confused - we use react-ace,

react-ace depends on brace though. I think that's what they use to add Ace. This issue is about trying to get rid of it. So they might be stuck as well.

Joplin uses an older version of react-ace, maybe upgrading to the latest might resolve some of the issues.

@tessus

This comment has been minimized.

Copy link
Collaborator

@tessus tessus commented May 18, 2019

I changed the label question to question-w4f so that it s perfectly clear that we wait for input from someone.

@tessus

This comment has been minimized.

Copy link
Collaborator

@tessus tessus commented May 18, 2019

We still have to work through the sections:

  • Duplicates
  • Ace editor
  • Are there plans to move to a different editor?
@wxiaoguang

This comment has been minimized.

Copy link

@wxiaoguang wxiaoguang commented May 20, 2019

I am interested in WYSIWYG editor.

For WYSIWYG, currently there's no plan to support it. If a good open source wysiwyg JS editor ever exists we can reconsider, but for now every time I review the existing options, I can't find any that would allow the customisations that we would need.

Can you share your thoughts about the customizations we need and what are the problems with existing options?

@i-need-to-tell-you-something

This comment has been minimized.

Copy link

@i-need-to-tell-you-something i-need-to-tell-you-something commented May 23, 2019

@innocuo #862 looks like a valid issue. I won't close this one.

To me this seems to be working as requested in that issue. Can someone else verify?

@tessus

This comment has been minimized.

Copy link
Collaborator

@tessus tessus commented May 23, 2019

@i-need-to-tell-you-something I think the main part in #862 has been fixed, but there was a second part to it.. Alternativelly, there could be a button to enable/disable search term highlighting.

Ok, when browsing through it I missed the _Alternativelly . In that case I can close it.

@foxmask

This comment has been minimized.

Copy link
Collaborator

@foxmask foxmask commented Jun 19, 2019

as it's possible to use an external editor and joplin won't cover the needs of everybody ; what ever it will be done ; can't we close all the issues from the "Are there plans to move to a different editor?" paragraph and then suggest the users to use external editor ?

@laurent22 laurent22 unpinned this issue Sep 4, 2019
@stale

This comment has been minimized.

Copy link

@stale stale bot commented Sep 21, 2019

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as "backlog" and I will leave it open. Thank you for your contributions.

@stale stale bot added the stale label Sep 21, 2019
@stale

This comment has been minimized.

Copy link

@stale stale bot commented Sep 28, 2019

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.

@stale stale bot closed this Sep 28, 2019
@lock lock bot locked and limited conversation to collaborators Oct 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.