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

Create trix-editor in project #63

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Create trix-editor in project #63

wants to merge 6 commits into from

Conversation

Apachina
Copy link
Collaborator

@Apachina Apachina commented Aug 27, 2018

Создавая пуллреквест, я:

  • убедился, что diff измненеий содержит только внесеные мной изменения
  • проверил, что закоментированный код удален (это касается именно закоментированного кода, а не настроек гемов)
  • проверил, что PR не содержит пустых ненужных файлов
  • запустил ./checkers и rspec
  • проверил именование переменных и методов и их соответвие соглашениям

@@ -14,4 +14,4 @@
//= require popper
//= require bootstrap-sprockets
//= require activestorage
//= require_tree .
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test how this deletion influences on user interface?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

))))

@@ -13,6 +13,6 @@
*= require bootstrap_and_overrides
*= require static_page
*= require _colors
*= require_tree .
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Здесь тоже

@@ -0,0 +1 @@
<%= field.to_s %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Разве без to_s не работает?
По-моему, оно само приводит

Copy link
Collaborator Author

@Apachina Apachina Aug 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Данные вещи гинерил administrate, при создании нового field'a

Gemfile Outdated
@@ -28,6 +28,7 @@ gem "simple_form", "4.0.1"
gem "administrate", "0.10.0"
gem "will_paginate", "3.1.6"
gem "administrate-field-carrierwave", "0.3.2"
gem 'trix', "0.11.1"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Эта версия не совместима. Судя по всему, у тебя опечатка. У меня по дефолту стала 0.10.1

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trix 0.11.1

на рубигемах... Прикольно)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Проверь, пожалуйста, работает ли у тебя на 0.11.1 и у других?
Я просто пулил и выдало ошибку.

Copy link
Collaborator

@mrgrecha mrgrecha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У тебя ещё вся вёрстка полетела, если зайти, как пользователь :)
Исправь.

//= require cable
//= require static_page
//= require cable
//= require trix
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add last line

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.

3 participants