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

improve review card on /reviews #384

Merged
merged 2 commits into from Apr 27, 2023
Merged

Conversation

liz4chernyshova
Copy link
Contributor

Изменила карточку отзыва: выводится дата публикации, слово "курс" входит в ссылку, добавила сортировку по дате публикации

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.

Всё хорошо, но надо переделать

@@ -2,6 +2,6 @@

class Web::ReviewsController < Web::ApplicationController
def index
@reviews = Review.published.with_locale.page(params[:page])
@reviews = Review.published.with_locale.page(params[:page]).order(created_at: :desc)
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.

это не так работает
после page у тебя по прежнему объект запроса и можно добавлять условия и сортировки,а не результат

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

@amshkv amshkv Apr 27, 2023

Choose a reason for hiding this comment

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

но сортировки и всё такое разве будет работать ко всем даннным? а не только к тому, что в пагинацию попало?

- let link_to review.language, language_path(review.language.slug), class: 'text-body' do |link|
.small.mb-2 = t('.course_html', link: link)
.small.mb-2
= link_to t('.course_html', link: review.language), language_path(review.language.slug), class: 'text-body'
Copy link
Contributor

Choose a reason for hiding this comment

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

от тут какая-то херня получилась с переводом, почему это html?
и почему передающаяся переменная это link?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Андрей, немного не понимаю замечания
link переименовать на name или languege, понятно
а с html что не так?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

А, поняла

@amshkv amshkv merged commit 4ddfc90 into main Apr 27, 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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants