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

Add markdown editor in the card description when editting #333

Merged
merged 5 commits into from
Nov 20, 2022
Merged

Add markdown editor in the card description when editting #333

merged 5 commits into from
Nov 20, 2022

Conversation

raflymln
Copy link
Contributor

Since the card support markdown render, it'd be great if we also had markdown editor so for those who doesn't understand a lot about markdown could use this feature.

With this new feature, there also a drawbacks such as no longer saving when input are onBlur since if we still using this blur callback, it'd be called when user trying to click at the editor toolbar such as Bold button.

image
image

Since the card support markdown render, it'd be great if we also had markdown editor so for those who doesn't understand a lot about markdown could use this feature.

With this new feature, there also a drawbacks such as no longer saving when input are onBlur since if we still using this `blur` callback, it'd be called when user trying to click at the editor toolbar such as **Bold** button.
@raflymln
Copy link
Contributor Author

Also i have some suggestion for the eslint and prettier settings, i think it'd be great if we add more tab spacing and print width in the prettier so you can read the code more better

Here's the current configuration:

{
    "printWidth": 100,
    "singleQuote": true,
    "trailingComma": "all",
}

image

My proposed settings:

{
    "printWidth": 200,
    "singleQuote": true,
    "trailingComma": "all",
    "tabWidth": 4
}

image

@raflymln
Copy link
Contributor Author

And if we can change eslint no-unused-vars settings to "warn" i think that would be great to speed up development, since you don't need to handle with unused vars while developing (adding/removing) features that requires a lot of variables

I think it'd be better if we clean up those mess in the end before releasing into production, wdyt?

Proposed eslint rules:

"no-unused-vars": "warn"

image

With this feature, any unused vars would not block front-end to rendering the component

@raflymln raflymln mentioned this pull request Nov 19, 2022
@meltyshev
Copy link
Member

Hi! It's a cool addition 👍
But unfortunately some elements of the editor don't work, such as Preview or Side by Side. It would also be great to remove the padding.

Regarding eslint and prettier settings, I don't think it would work for me because of the 13-inch screen. We should probably think about how to set it up so that we can work locally with other settings...

Regarding the "no-unused-vars: "warn" I completely agree, I always have to comment code with unused variables and it's annoying.

@raflymln
Copy link
Contributor Author

Alright so then i'll edit up the eslint settings and also i'll remove some unused features from the markdown editor, what do you think about that?

@meltyshev
Copy link
Member

For me, the perfect view would be:

Screenshot 2022-11-19 at 19 36 48

I also forgot to change the color of the font in the editor, but maybe it doesn't matter for the first version 🙈

Regarding eslint, it's better to do this in another PR or I can add it myself.

- Remove padding
- Remove border

Most of the stylings had been done within its React SimpleMDE Package
@raflymln
Copy link
Contributor Author

Newest commit result
image

@meltyshev meltyshev merged commit 5dcbad7 into plankanban:master Nov 20, 2022
@meltyshev
Copy link
Member

Perfect! Thank you 🙏

@raflymln raflymln deleted the enhancement/markdown-editor branch November 20, 2022 13:49
@johnnyduncan
Copy link

Wow, good work!!

@raflymln
Copy link
Contributor Author

Perfect! Thank you 🙏
Wow, good work!!

Thank you so much 🙏

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.

None yet

3 participants