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

steps admin panel #610

Merged
merged 9 commits into from Jul 5, 2023
Merged

steps admin panel #610

merged 9 commits into from Jul 5, 2023

Conversation

usernaimandrey
Copy link
Contributor

@usernaimandrey usernaimandrey commented Jun 16, 2023

Задачи:

  1. Вынести список шагов и их редактирование в отдельный роут
  2. В интерфейсе редактирования шага добавит ссылки на КТ в которых он есть
  3. Сделать кнопки редактирования и просмотра явными
  4. Показывать на show шага как он будет отображаться и в каких КТ находится этот шаг

@usernaimandrey usernaimandrey marked this pull request as ready for review June 19, 2023 07:57
@usernaimandrey
Copy link
Contributor Author

image
3.
image
4.
image

Comment on lines 17 to 19
= link_to edit_admin_career_step_path(step), class: 'btn btn-outline-primary btn-sm me-1', title: t('.edit') do
span.bi.bi-gear-fill
= link_to admin_career_step_path(step), class: 'btn btn-outline-primary btn-sm', title: t('.show') do
Copy link
Contributor

Choose a reason for hiding this comment

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

порядок кнопок не тот, обычно
посмотреть \ редактировать \ удалить

@@ -0,0 +1,20 @@
= render 'menu'
Copy link
Contributor

Choose a reason for hiding this comment

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

а заголовка всё-таки не хватает

@@ -0,0 +1,3 @@
ul.nav.nav-tabs.mb-4
= menu_item t('.list'), admin_career_steps_path
= menu_item t('.new'), new_admin_career_step_path
Copy link
Contributor

Choose a reason for hiding this comment

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

на Хекслете мы стараемся делать меню во всех сущностях, в том числе и в редактировании, чтобы был понятный переход где ты находишься
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

Copy link
Contributor

Choose a reason for hiding this comment

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

давай слово Шаг добавим в заголовок
в меню можно оставить "редактирование", у нас на Хекслете вроде так?
по выравниванию можно @richpeach-bot попросить глянуть одним глазом

Copy link
Contributor

Choose a reason for hiding this comment

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

@usernaimandrey, я бы сохраняла для этих бейджей логику всей формы: лейбл слева в колонке, содержимое -- справа в колонке

Comment on lines 4 to 7
- if @careers.any?
= link_to '#attached', class: 'btn btn-primary' do
span.bi.bi-eye.me-1
= t('.attached_careers')
Copy link
Contributor

Choose a reason for hiding this comment

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

ну ваще не в кассу огромная синяя кнопка
может в шапке (или под заголовком) расположить small-ссылки-баджи?
можно попросить помочь Яну

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Убрал кнопку сделал бейджики
image


= render 'form', step: @step, url: admin_career_step_path(@step)

= render 'careers', careers: @careers
Copy link
Contributor

Choose a reason for hiding this comment

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

внизу выглядит ОК,
но надо бы как-то отбить от основного контента (или вообще не показывать)

Copy link
Contributor

Choose a reason for hiding this comment

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

image если сделать аккуратно, то можно убрать

Copy link
Contributor

@amshkv amshkv left a comment

Choose a reason for hiding this comment

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

ждем исправления визуала и попросим Стаса проверить путь

td = l step.created_at, format: :long
td = link_to t('.edit'), edit_admin_career_step_path(career, step), class: 'btn btn-primary'
td = link_to t('.edit'), edit_admin_career_step_path(step), class: 'btn btn-primary'
Copy link
Contributor

Choose a reason for hiding this comment

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

наверное лучше target _blank

= link_to admin_career_path(career), class: 'btn btn-outline-primary btn-sm', title: t('.show') do
span.bi.bi-eye-fill
- else
= render partial: 'web/shared/empty_list'
Copy link
Contributor

Choose a reason for hiding this comment

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

partial можно опустить

@usernaimandrey
Copy link
Contributor Author

@amshkv Можно чекать)

@@ -0,0 +1,20 @@
h4 = t('.attached')
Copy link
Contributor

Choose a reason for hiding this comment

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

такие вещи лучше делать без семантики
через .h4 класс или fs-4

- content_for :header do
= t('.new_step')

= render 'menu', step: nil
Copy link
Contributor

Choose a reason for hiding this comment

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

чтобы не писать везде nil мы на хекслете вот так делали
image
но тут Стас наверное подскажет как лучше и правильнее

Comment on lines 9 to 15
h3 = @step.name
.bg-light.rounded.py-2.px-3
== markdown2html(@step.description)
.my-1
h4 = t('.tasks')
.bg-light.rounded.py-2.px-3
== markdown2html(@step.tasks_text)
Copy link
Contributor

Choose a reason for hiding this comment

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

давай уберем семантически заголовки на просто заголовки

@@ -1,6 +1,6 @@
h3.my-4.me-3 = t('.steps_career')

= link_to t('.add_new_step'), new_admin_career_step_path(career), class: 'btn btn-primary'
/ = link_to t('.add_new_step'), new_admin_career_step_path(career), class: 'btn btn-primary'
Copy link
Contributor

Choose a reason for hiding this comment

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

это временно закомментили чтобы вернуть если что?

config/routes.rb Outdated
@@ -96,9 +96,10 @@
end
end
resources :vacancies, only: %i[index edit update]
resources :career_steps, only: %i[index new show create edit update]
Copy link
Contributor

Choose a reason for hiding this comment

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

А тут решили делать именно отдельным ресурсом, не вложенным на коллекции?

/careers/steps
/careers/steps/:id
...

Вроде всё же не предполагается что шаг может использоваться с какой-нибудь другой сущностью? Собственно и в моделях step - Career::Step
То есть step по сути это вложенный ресурс, но для всех треков.

@@ -11,14 +18,14 @@ def new

def edit
@step = Career::Step.find(params[:id])
@careers = @step.careers
Copy link
Contributor

Choose a reason for hiding this comment

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

А в методе update если упало с ошибками валидации нам не нужно во вьюху передавать @careers

@usernaimandrey usernaimandrey merged commit 5f74437 into main Jul 5, 2023
1 check passed
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

5 participants