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

Edit article + CSP #12

Merged
merged 20 commits into from
Nov 19, 2018
Merged

Edit article + CSP #12

merged 20 commits into from
Nov 19, 2018

Conversation

Reeywhaar
Copy link
Collaborator

Редактирование новости

Сделал возможность редактирования новости:
http://rtnews.vyrtsev.com/post/agile-won-the-war-but-lost-the-peace под заголовком есть псевдоссылка "редактировать" если залогинен.
Заметил, что при обновлении новости у неё непонятным образом меняется приоритет, она то уходит в середину списка, то появляется вверху.


CSP

Добавил CSP заголовок чтобы уж точно никакой скрипт посторонний не выполнился, но надо иметь его ввиду при изменении скрипта гиттера или добавлении других посторонних скриптов.

@umputun
Copy link
Member

umputun commented Nov 19, 2018

редатирование это довольно редкая фуннциональность, я не думаю что для этого надо везде показывать сниппет

u4oyk_20181118_215506 cid9q

ссылка редактировния, которая занимает целую строку, тоже кажется слишком расточительной для такой экзотики

@Reeywhaar
Copy link
Collaborator Author

Reeywhaar commented Nov 19, 2018

Сниппет показывается только если залогинен, и нужен, чтобы после редактирования сразу быстро посмотреть на результат. То, есть без него как-то странно.

@umputun
Copy link
Member

umputun commented Nov 19, 2018

все ведущие залогинены всегда, но показывать им все снипет для поддержки такой редкой и не очень важной функции - это перебор. Если для того, чтоб это спрятать надо отказаться от preview – я ничего против не имею. Вообще я не предполагал тут никакой возможности вводить и сниппет и текст, как впрочем и rich editor который меня удивил. Все что я хотел - это по нажатию какой малозаметной конпки edit ввести немного текста который пойдет и в снипет и в текст.

В принципе, я ничего не имею против продвинутого редкатора, так можно действительно не только ввести текст но и поправить (я о таком не думал, в принципе можно представить себе зачем оно надо), но все это получается как-то слишком сложно и громоздко.

@Reeywhaar
Copy link
Collaborator Author

Да, wysiwig я встроил, потому что имеющиеся новости уже в хтмл, и если редактировать в простом поле ввода, то надо как-то деконструировать хтмл в простой текст, что не может быть сделано очевидно и однозначно. В принципе я сделал lazy-loading всего этого дела и он никак не влияет на загрузку пока не нажмешь "редактировать"

@umputun
Copy link
Member

umputun commented Nov 19, 2018

при обновлении новости у неё непонятным образом меняется приоритет

Может ты не посылаешь position при сохранении ?

@Reeywhaar
Copy link
Collaborator Author

Может ты не посылаешь position при сохранении ?

А надо всё посылать? Я посылаю title, link, content и snippet.

@umputun
Copy link
Member

umputun commented Nov 19, 2018

да, надо все. этот вызов не знает что именно ты обновляешь

@umputun
Copy link
Member

umputun commented Nov 19, 2018

он никак не влияет на загрузку пока не нажмешь "редактировать"

меня не загрузка волнует, но визуальный "шум", который нужен только в момент подготовки новостей (и очень редко), и совершенно не нужен в процессе проведения подкаста

@Reeywhaar
Copy link
Collaborator Author

Да, проблема с приоритетом решилась.

@Reeywhaar
Copy link
Collaborator Author

Надо наверное правда сделать какое-то совсем простое пустое поле, прямо на главной странице, куда можно по-быстрому вписать текст.

@umputun
Copy link
Member

umputun commented Nov 19, 2018

так как сейчас нормально, ничего не лезет в глаза. Только странная двойная рамка выглядит как баг
711p8-201811-18225628-ej95d

@Reeywhaar
Copy link
Collaborator Author

поправил

@umputun umputun merged commit 0ff0ea3 into radio-t:master Nov 19, 2018
@Reeywhaar Reeywhaar deleted the edit-article branch December 19, 2018 02:14
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.

2 participants