Plume
/
Plume
8
16
Atdalīts 22
Kods Problēmas 174 Izmaiņu pieprasījumi 8 Laidieni 8 Vikivietne Aktivitāte

Etiķetes Atskaites punkti

New editor #458

Sapludināts
elegaanz sapludināja 13 revīzijas no editor uz master pirms 5 gadiem
Saruna 8 Revīzijas 13 Izmainītie faili 25
elegaanz " komentēja pirms 5 gadiem" (Migrēts no github.com)

With this PR, when JS is activated and WASM supported, the article editor will be dynamically replaced with contenteditables elements. This makes the editing interface simpler and less like a regular form. It will also allow us to easily add visual formatting with native browser APIs (and to insert images or videos directly). Here is a little demo:

peek 05-03-2019 16-12

There is still a lot to do, but it is a good first step.

Fixes #255

With this PR, when JS is activated and WASM supported, the article editor will be dynamically replaced with `contenteditable`s elements. This makes the editing interface simpler and less like a regular form. It will also allow us to easily add visual formatting with native browser APIs (and to insert images or videos directly). Here is a little demo: ![peek 05-03-2019 16-12](https://user-images.githubusercontent.com/16254623/53815536-1dc05680-3f62-11e9-94d3-b363ed84eb97.gif) There is still a lot to do, but it is a good first step. Fixes #255
codecov[bot] " komentēja pirms 5 gadiem" (Migrēts no github.com)

Codecov Report

Merging #458 into master will increase coverage by 1.64%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #458      +/-   ##
==========================================
+ Coverage   26.49%   28.13%   +1.64%     
==========================================
  Files          64       64              
  Lines        7111     7496     +385     
==========================================
+ Hits         1884     2109     +225     
- Misses       5227     5387     +160
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/458?src=pr&el=h1) Report > Merging [#458](https://codecov.io/gh/Plume-org/Plume/pull/458?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/2a188abfa152efe7a2794174279e3b9da4a809d1?src=pr&el=desc) will **increase** coverage by `1.64%`. > The diff coverage is `n/a`. ```diff @@ Coverage Diff @@ ## master #458 +/- ## ========================================== + Coverage 26.49% 28.13% +1.64% ========================================== Files 64 64 Lines 7111 7496 +385 ========================================== + Hits 1884 2109 +225 - Misses 5227 5387 +160 ```
elegaanz " komentēja pirms 5 gadiem" (Migrēts no github.com)

Something I just noticed: published articles are always drafts (unless you are editing an already published article)

Something I just noticed: published articles are always drafts (unless you are editing an already published article)
trinity-1686a " komentēja pirms 5 gadiem"
Īpašnieks

I'm having trouble making it work. I had to tinker plume-front.js. Any idea why, or how to prevent that?

I'm having trouble making it work. I had to tinker plume-front.js. Any idea why, or how to prevent that?
elegaanz " komentēja pirms 5 gadiem" (Migrēts no github.com)

Was the build script correctly run by cargo? Like, did you saw plume(build) before plume(bin) next to the compilation progress bar? Otherwise, you can try to add a println!("cargo:rerun-if-changed=target/deploy/plume-front.js") (IIRC) to it, so that we are sure it is re-run when the JS changes (it does when the WASM changes normally, but maybe that's not enough).

Was the build script correctly run by cargo? Like, did you saw `plume(build)` before `plume(bin)` next to the compilation progress bar? Otherwise, you can try to add a `println!("cargo:rerun-if-changed=target/deploy/plume-front.js")` (IIRC) to it, so that we are sure it is re-run when the JS changes (it does when the WASM changes normally, but maybe that's not enough).
trinity-1686a " komentēja pirms 5 gadiem"
Īpašnieks

ok my bad, I ran cargo web deploy -o ../static so the buildscript did not update the js file as it should have

ok my bad, I ran `cargo web deploy -o ../static` so the buildscript did not update the js file as it should have
👍 1
trinity-1686a recenzēja pirms 5 gadiem
plume-front/src/editor.rs
trinity-1686a " komentēja pirms 5 gadiem"
Īpašnieks

By doing the following change, I was able to copy past a picture in a post and have it rendered as expected. However when re-editing the article, < and > where escaped, leading to a messy html tag in the middle of the content. I don't know if this is considered in scope for this pr, but this might be something to look at, as it will get important once the editor get richer

                            set_value("editor-content", js!{return @{&widgets.2}.innerHTML}.as_str().unwrap_or_default());
By doing the following change, I was able to copy past a picture in a post and have it rendered as expected. However when re-editing the article, `<` and `>` where escaped, leading to a messy html tag in the middle of the content. I don't know if this is considered in scope for this pr, but this might be something to look at, as it will get important once the editor get richer ```suggestion set_value("editor-content", js!{return @{&widgets.2}.innerHTML}.as_str().unwrap_or_default()); ```
trinity-1686a recenzēja pirms 5 gadiem
po/plume-front/en.po~
trinity-1686a " komentēja pirms 5 gadiem"
Īpašnieks

should this file (and fr.po~) exists?

should this file (and fr.po~) exists?
igalic (Migrēts no github.com) recenzēja pirms 5 gadiem
po/plume-front/en.po~
igalic (Migrēts no github.com) " komentēja pirms 5 gadiem"

my suggestion to @BaptisteGelez is to either add this our .gitignore or to her global .git/ignore

my suggestion to @BaptisteGelez is to either add this our `.gitignore` or to her global `.git/ignore`
elegaanz (Migrēts no github.com) recenzēja pirms 5 gadiem
po/plume-front/en.po~
elegaanz (Migrēts no github.com) " komentēja pirms 5 gadiem"

No, I though *.po~ was in the gitignore, but it looks like it is not. 🤷‍♀️ I will add it.

No, I though `*.po~` was in the gitignore, but it looks like it is not. :woman_shrugging: I will add it.
elegaanz (Migrēts no github.com) recenzēja pirms 5 gadiem
plume-front/src/editor.rs
elegaanz (Migrēts no github.com) " komentēja pirms 5 gadiem"

I think it would be easy to avoid escaping here, I will try.

I think it would be easy to avoid escaping here, I will try.
trinity-1686a recenzēja pirms 5 gadiem
po/plume-front/en.po~
trinity-1686a " komentēja pirms 5 gadiem"
Īpašnieks

They are only ignored in some directories, not everywhere

They are only ignored in some directories, not everywhere
elegaanz " komentēja pirms 5 gadiem" (Migrēts no github.com)

I added back the character counter, but it is not as precise as the previous one, since I only took the article content in account (otherwise I would have had to rebuild all the original form every time, which would be quite long both in code and when running). I don't think that's a big issue, since the previous one was not perfectly precise neither, and that people will rarely reach this limit anyway.

I added back the character counter, but it is not as precise as the previous one, since I only took the article content in account (otherwise I would have had to rebuild all the original form every time, which would be quite long both in code and when running). I don't think that's a big issue, since the previous one was not perfectly precise neither, and that people will rarely reach this limit anyway.
trinity-1686a recenzēja pirms 5 gadiem
trinity-1686a atstāja komentāru
Īpašnieks

mostly code-style comment again

mostly code-style comment again
.gitignore
trinity-1686a " komentēja pirms 5 gadiem"
Īpašnieks

this line is duplicated

this line is duplicated
plume-front/src/editor.rs
trinity-1686a " komentēja pirms 5 gadiem"
Īpašnieks

this closure is really long. It would gain in readability to be divided in multiple smaller functions called one after an other (like "hide non-js form", "add js editor", "show popup", "copy editor to form"...). I understand what it's doing, but if there is something wrong, it's too complex to see it right away

this closure is really long. It would gain in readability to be divided in multiple smaller functions called one after an other (like "hide non-js form", "add js editor", "show popup", "copy editor to form"...). I understand what it's doing, but if there is something wrong, it's too complex to see it right away
plume-front/src/main.rs
@ -6,0 +20,4 @@
lazy_static! {
static ref CATALOG: gettext::Catalog = {
let catalogs = include_i18n!();
let lang = js!{ return navigator.language }.into_string().unwrap();
trinity-1686a " komentēja pirms 5 gadiem"
Īpašnieks

I love that it just works like in the back-end, with nothing more to

I love that it just works like in the back-end, with nothing more to
elegaanz (Migrēts no github.com) recenzēja pirms 5 gadiem
plume-front/src/main.rs
@ -6,0 +20,4 @@
lazy_static! {
static ref CATALOG: gettext::Catalog = {
let catalogs = include_i18n!();
let lang = js!{ return navigator.language }.into_string().unwrap();
elegaanz (Migrēts no github.com) " komentēja pirms 5 gadiem"

Yeah, that's really great! The only downside is that we include all the translations in the .wasm so it is a bit bigger, but there is like 3 or 4 messages for the moment so I guess it's fine (and I don't expect it to become much bigger, I think we will never have more than 30 messages for the front end).

Yeah, that's really great! The only downside is that we include all the translations in the .wasm so it is a bit bigger, but there is like 3 or 4 messages for the moment so I guess it's fine (and I don't expect it to become much bigger, I think we will never have more than 30 messages for the front end).
trinity-1686a recenzēja pirms 5 gadiem
plume-front/src/main.rs
@ -6,0 +20,4 @@
lazy_static! {
static ref CATALOG: gettext::Catalog = {
let catalogs = include_i18n!();
let lang = js!{ return navigator.language }.into_string().unwrap();
trinity-1686a " komentēja pirms 5 gadiem"
Īpašnieks

considering all Plume translations (not plume-front) are 208k on my computer, any picture loaded on the front page must cost much more than having all translations available

considering all Plume translations (not plume-front) are 208k on my computer, any picture loaded on the front page must cost much more than having all translations available
elegaanz " komentēja pirms 5 gadiem" (Migrēts no github.com)

I tried to make the code a bit more readable, but I'm not sure it is enough…

I tried to make the code a bit more readable, but I'm not sure it is enough…
igalic (Migrēts no github.com) recenzēja pirms 5 gadiem
plume-front/src/main.rs
@ -6,0 +20,4 @@
lazy_static! {
static ref CATALOG: gettext::Catalog = {
let catalogs = include_i18n!();
let lang = js!{ return navigator.language }.into_string().unwrap();
igalic (Migrēts no github.com) " komentēja pirms 5 gadiem"

so how big is the dynamic front-end right now? and how much of that is cachable?

so how big is the dynamic front-end right now? and how much of that is cachable?
elegaanz (Migrēts no github.com) recenzēja pirms 5 gadiem
plume-front/src/main.rs
@ -6,0 +20,4 @@
lazy_static! {
static ref CATALOG: gettext::Catalog = {
let catalogs = include_i18n!();
let lang = js!{ return navigator.language }.into_string().unwrap();
elegaanz (Migrēts no github.com) " komentēja pirms 5 gadiem"

It is 2.2M on my computer, but there are various ways to optimize WASM size that we didn't explored yet. And I don't know how much of it is cachable? Nothing?

It is 2.2M on my computer, but there are various ways to optimize WASM size that we didn't explored yet. And I don't know how much of it is cachable? Nothing?
trinity-1686a recenzēja pirms 5 gadiem
plume-front/src/main.rs
@ -6,0 +20,4 @@
lazy_static! {
static ref CATALOG: gettext::Catalog = {
let catalogs = include_i18n!();
let lang = js!{ return navigator.language }.into_string().unwrap();
trinity-1686a " komentēja pirms 5 gadiem"
Īpašnieks

I'd say everything, it's a static file after all. I tried compiling with cargo web deploy --release, it seems to work, and is "only" 832k. We could also try to compact plume-front.js as it's 35k with indentation and things, however I'm not sure it's that important

I'd say everything, it's a static file after all. I tried compiling with `cargo web deploy --release`, it seems to work, and is "only" 832k. We could also try to compact plume-front.js as it's 35k with indentation and things, however I'm not sure it's that important
trinity-1686a apstiprināja izmaiņas pirms 5 gadiem
trinity-1686a atstāja komentāru
Īpašnieks

👍

:+1:
trinity-1686a " komentēja pirms 5 gadiem"
Īpašnieks

Oups, 2 little things : It doesn't seems to be possible to add carriage return in the content (prevent default I guess), and it may be nice to have a button to switch back to the plain markdown editor

Oups, 2 little things : It doesn't seems to be possible to add carriage return in the content (prevent default I guess), and it may be nice to have a button to switch back to the plain markdown editor

Recenzenti

trinity-1686a apstiprināja izmaiņas pirms 5 gadiem
Izmaiņu pieprasījums tika sapludināts ar revīziju 9076dbaadc.
Varat aplūkot arī komandrindas instrukcijas.

Solis 1:

Projekta repozitorijā izveidojiet jaunu jaunu atzaru un pārbaudiet savas izmaiņas.
git checkout -b editor master
git pull origin editor

Solis 2:

Sapludināt izmaiņas un atjaunot tās Forgejo.
git checkout master
git merge --no-ff editor
git push origin master
Pierakstieties, lai pievienotos šai sarunai.
Recenzenti
Pieprasīt recenziju
Nav recenzentu
trinity-1686a
Etiķetes
Apstiprināt etiķetes
Noņemt etiķetes   
A: API

Related to the REST API   
A: Backend

Code running on the server   
A: Federation

Stuff related to Federation   
A: Front-End

Related to the front-end   
A: I18N

Translations, and related code   
A: Meta

More about project management or code than the project itself   
A: Security
   
Build

The building, or installation process of Plume   
C: Bug

Something isn't working   
C: Discussion

We need to talk   
C: Enhancement

New feature or request   
C: Feature

This is a new feature   
Compatibility

Compatibility with different browsers, readers and OS   
Dependency

Related to an external package that Plume uses   
Design

UI/UX related issues and PRs   
Documentation
   
Good first issue

Good for newcomers   
Help welcome

Extra attention is needed   
Mobile

Issues affecting only mobile UX   
Rendering

How elements're rendered out for the end user   
S: Blocked

Something else needs to be fixed first   
S: Duplicate

This issue or pull request already exists   
S: Incomplete

This PR is not complete yet   
S: Instance specific

Issues concern a limited number of instances   
S: Invalid

This doesn't seem right   
S: Needs Voting/Discussion

Need to be discussed by the community (on Loomio)   
S: Ready for review

This PR is ready to be reviewed   
Suggestion

Proposed ideas worth considering   
S: Voted on Loomio

This is issue has been created after a vote on Loomio   
S: Wontfix

This will not be worked on
Nav etiķešu
A: API
A: Backend
A: Federation
A: Front-End
A: I18N
A: Meta
A: Security
Build
C: Bug
C: Discussion
C: Enhancement
C: Feature
Compatibility
Dependency
Design
Documentation
Good first issue
Help welcome
Mobile
Rendering
S: Blocked
S: Duplicate
S: Incomplete
S: Instance specific
S: Invalid
S: Needs Voting/Discussion
S: Ready for review
Suggestion
S: Voted on Loomio
S: Wontfix
Atskaites punkts
Uzstādīt atskaites punktu
Notīrīt atskaites punktus
Nav neviena ieraksta
Nav atskaites punktu
Alpha 2
Atbildīgie
Piešķirt lietotājus
Noņemt atbildīgo
Nav atbildīgo
2 dalībnieki
Paziņojumi
Izpildes termiņš
Datums līdz nav korekts. Izmantojiet formātu 'gggg-mm-dd'.

Izpildes termiņš nav uzstādīts.

Atkarības

Nav atkarību.

Atsaucas uz: Plume/Plume#458
Rakstīt Priekšskatītījums
Notiek ielāde…
Atcelt
Saglabāt
Vēl nav satura.
Dzēst atzaru 'editor'

Atzara dzēšana ir neatgriezeniska, kā arī tā ir NEATGRIEZENISKA. Vai turpināt?

Nē
Jā