Skip to content

Commit

Permalink
implemented feedback from product team, finalized new specs, cleanup,…
Browse files Browse the repository at this point in the history
… bugfixing
  • Loading branch information
jjabari-op committed Aug 20, 2024
1 parent 8b138e8 commit 0f276c7
Show file tree
Hide file tree
Showing 15 changed files with 616 additions and 199 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
WorkPackages::ActivitiesTab::Journals::IndexComponent.new(work_package:, filter:)
)
end
journals_wrapper_container.with_row(id: "stem-connection")
journals_wrapper_container.with_row(
id: "input-container",
classes: "sort-#{journal_sorting}", # css order attribute will take care of correct position
Expand Down
12 changes: 12 additions & 0 deletions app/components/work_packages/activities_tab/index_component.sass
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#work-packages-activities-tab-index-component
#journals-container
z-index: 10
padding-top: 3px
overflow-y: auto
&.with-initial-input-compensation
Expand All @@ -10,7 +11,18 @@
margin-bottom: 180px
@media screen and (max-width: $breakpoint-xl)
margin-bottom: -16px
#stem-connection
@media screen and (min-width: $breakpoint-xl)
position: absolute
z-index: 9
border-left: var(--borderWidth-thin, 1px) solid var(--borderColor-default)
margin-left: 19px
margin-top: 20px
height: 100vh
@media screen and (max-width: $breakpoint-xl)
display: none
#input-container
z-index: 10
@media screen and (min-width: $breakpoint-xl)
position: absolute
min-height: 60px
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<%=
component_wrapper do
render(Primer::Beta::Blankslate.new(border: true, data: { test_selector: "op-wp-journals-container-empty "})) do |component|
component.with_visual_icon(icon: :pulse)
component.with_heading(tag: :h2).with_content(t("activities.work_packages.activity_tab.no_results_title_text"))
component.with_description { t("activities.work_packages.activity_tab.no_results_description_text") }
end
end
%>
Original file line number Diff line number Diff line change
Expand Up @@ -31,30 +31,13 @@
module WorkPackages
module ActivitiesTab
module Journals
class DayComponent < ApplicationComponent
class EmptyComponent < ApplicationComponent
include ApplicationHelper
include OpPrimer::ComponentHelpers
include OpTurbo::Streamable

def initialize(day_as_date:, journals:, work_package:, filter:)
def initialize
super

@work_package = work_package
@day_as_date = day_as_date
@journals = journals
@filter = filter
end

private

attr_reader :work_package, :day_as_date, :journals, :filter

def insert_target_modified?
true
end

def insert_target_modifier_id
"work-package-journals-day-#{day_as_date}"
end

Check warning on line 41 in app/components/work_packages/activities_tab/journals/empty_component.rb

View workflow job for this annotation

GitHub Actions / rubocop

[rubocop] app/components/work_packages/activities_tab/journals/empty_component.rb#L39-L41 <Lint/UselessMethodDefinition>

Useless method definition detected.
Raw output
app/components/work_packages/activities_tab/journals/empty_component.rb:39:9: W: Lint/UselessMethodDefinition: Useless method definition detected.

Check notice on line 41 in app/components/work_packages/activities_tab/journals/empty_component.rb

View workflow job for this annotation

GitHub Actions / rubocop

[rubocop] app/components/work_packages/activities_tab/journals/empty_component.rb#L39-L41 <Style/RedundantInitialize>

Remove unnecessary `initialize` method.
Raw output
app/components/work_packages/activities_tab/journals/empty_component.rb:39:9: C: Style/RedundantInitialize: Remove unnecessary `initialize` method.
end
end
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
<%=
component_wrapper do
flex_layout(id: insert_target_modifier_id, data: { "test_selector": "op-wp-journals-container" }) do |journals_index_container|
if journals_grouped_by_day.any?
journals_grouped_by_day.each do |day, journals|
journals_index_container.with_row do
render(
WorkPackages::ActivitiesTab::Journals::DayComponent.new(day_as_date: day, journals:, work_package:, filter:)
)
end
end
else
if filter == :only_comments && journal_with_notes.empty?
journals_index_container.with_row(mt: 2) do
render(Primer::Beta::Blankslate.new(border: true)) do |component|
component.with_visual_icon(icon: :pulse)
component.with_heading(tag: :h2).with_content(t("activities.work_packages.activity_tab.no_results_title_text"))
component.with_description { t("activities.work_packages.activity_tab.no_results_description_text") }
end
render(
WorkPackages::ActivitiesTab::Journals::EmptyComponent.new()
)
end
end
journals.each do |journal|
journals_index_container.with_row do
render(
WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter:)
)
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ def journal_sorting
User.current.preference&.comments_sorting || "desc"
end

def journals_grouped_by_day
result = work_package.journals.includes(:user, :notifications).reorder(version: journal_sorting)

result = result.where.not(notes: "") if filter == :only_comments
def journals
work_package.journals.includes(:user, :notifications).reorder(version: journal_sorting)
end

result.group_by { |journal| journal.created_at.in_time_zone(User.current.time_zone).to_date }
def journal_with_notes
journals.where.not(notes: "")
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
end
end
header_end_container.with_column(ml: 1) do
render(Primer::Alpha::ActionMenu.new) do |menu|
render(Primer::Alpha::ActionMenu.new(data: { "test_selector": "op-wp-journal-#{journal.id}-action-menu" })) do |menu|
menu.with_show_button(icon: "kebab-horizontal",
'aria-label': I18n.t(:button_actions),
scheme: :invisible)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def edit_action_item(menu)
menu.with_item(label: t("js.label_edit_comment"),
href: edit_work_package_activity_path(journal.journable, journal, filter:),
content_arguments: {
data: { "turbo-stream": true }
data: { "turbo-stream": true, test_selector: "op-wp-journal-#{journal.id}-edit" }
}) do |item|
item.with_leading_visual_icon(icon: :pencil)
end
Expand All @@ -131,7 +131,8 @@ def quote_action_item(menu)
data: {
action: "click->work-packages--activities-tab--index#quote",
"content-param": journal.notes,
"user-name-param": I18n.t(:text_user_wrote, value: ERB::Util.html_escape(journal.user))
"user-name-param": I18n.t(:text_user_wrote, value: ERB::Util.html_escape(journal.user)),
test_selector: "op-wp-journal-#{journal.id}-quote"
}
}) do |item|
item.with_leading_visual_icon(icon: :quote)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ def wrapper_uniq_by

def render_details_header(details_container)
details_container.with_row(flex_layout: true,
justify_content: :space_between, classes: "journal-details-header-container") do |header_container|
justify_content: :space_between,
classes: "journal-details-header-container",
id: "activity-#{journal.version}") do |header_container|
header_container.with_column(flex_layout: true,
classes: "journal-details-header") do |header_start_container|
header_start_container.with_column(mr: 2, classes: "timeline-icon") do
Expand Down
59 changes: 23 additions & 36 deletions app/controllers/work_packages/activities_tab_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,52 +197,32 @@ def generate_time_based_update_streams(last_update_timestamp, filter)

journals.where("updated_at > ?", last_update_timestamp).find_each do |journal|
update_via_turbo_stream(
# only use the show component in order not to loose an edit state
component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Show.new(
journal:,
filter:
)
)
update_via_turbo_stream(
# only use the show component in order not to loose an edit state
component: WorkPackages::ActivitiesTab::Journals::ItemComponent::Details.new(
# we need to update the whole component as the show part is not rendered for journals which originally have no notes
component: WorkPackages::ActivitiesTab::Journals::ItemComponent.new(
journal:,
filter:
)
)
# TODO: is it possible to loose an edit state this way?
end

latest_journal_visible_for_user = journals.where(created_at: ..last_update_timestamp).last

journals.where("created_at > ?", last_update_timestamp).find_each do |journal|
append_or_prepend_latest_journal_via_turbo_stream(journal, latest_journal_visible_for_user, filter)
append_or_prepend_latest_journal_via_turbo_stream(journal, filter)
end
end

def append_or_prepend_latest_journal_via_turbo_stream(journal, latest_journal, filter)
if latest_journal.created_at.to_date == journal.created_at.to_date
target_component = WorkPackages::ActivitiesTab::Journals::DayComponent.new(
work_package: @work_package,
day_as_date: journal.created_at.to_date,
journals: [journal], # we don't need to pass all actual journals of this day as we do not really render this component
filter:
)
component = WorkPackages::ActivitiesTab::Journals::ItemComponent.new(
journal:,
filter:
)
else
target_component = WorkPackages::ActivitiesTab::Journals::IndexComponent.new(
work_package: @work_package,
filter:
)
component = WorkPackages::ActivitiesTab::Journals::DayComponent.new(
work_package: @work_package,
day_as_date: journal.created_at.to_date,
journals: [journal],
filter:
)
if journals.any?
remove_potential_empty_state
end
end

def append_or_prepend_latest_journal_via_turbo_stream(journal, filter)
target_component = WorkPackages::ActivitiesTab::Journals::IndexComponent.new(
work_package: @work_package,
filter:
)

component = WorkPackages::ActivitiesTab::Journals::ItemComponent.new(journal:, filter:)

stream_config = {
target_component:,
component:
Expand All @@ -255,4 +235,11 @@ def append_or_prepend_latest_journal_via_turbo_stream(journal, latest_journal, f
prepend_via_turbo_stream(**stream_config)
end
end

def remove_potential_empty_state
# remove the empty state if it is present
remove_via_turbo_stream(
component: WorkPackages::ActivitiesTab::Journals::EmptyComponent.new
)
end
end
3 changes: 2 additions & 1 deletion app/forms/work_packages/activities_tab/journals/submit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
module WorkPackages::ActivitiesTab::Journals
class Submit < ApplicationForm
form do |notes_form|
notes_form.submit(name: :submit, label: "Save", scheme: :primary)
notes_form.submit(name: :submit, label: "Save", scheme: :primary,
data: { test_selector: "op-submit-work-package-journal-form" })
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,13 @@ export default class IndexController extends Controller {
declare workPackageIdValue:number;
declare localStorageKey:string;

private handleWorkPackageUpdateBound:EventListener;
private handleVisibilityChangeBound:EventListener;
private rescueEditorContentBound:EventListener;

connect() {
this.setLocalStorageKey();
this.setLastUpdateTimestamp();
this.hideLastPartOfTimeLineStem();
this.setupEventListeners();
this.handleInitialScroll();
this.startPolling();
Expand All @@ -57,21 +60,19 @@ export default class IndexController extends Controller {
}

private setupEventListeners() {
this.handleWorkPackageUpdate = this.handleWorkPackageUpdate.bind(this);
this.handleVisibilityChange = this.handleVisibilityChange.bind(this);
this.rescueEditorContent = this.rescueEditorContent.bind(this);
document.addEventListener('work-package-updated', this.handleWorkPackageUpdate);
document.addEventListener('visibilitychange', this.handleVisibilityChange);
document.addEventListener('beforeunload', this.rescueEditorContent);
this.handleWorkPackageUpdateBound = this.handleWorkPackageUpdate.bind(this);
this.handleVisibilityChangeBound = this.handleVisibilityChange.bind(this);
this.rescueEditorContentBound = this.rescueEditorContent.bind(this);

document.addEventListener('work-package-updated', this.handleWorkPackageUpdateBound);
document.addEventListener('visibilitychange', this.handleVisibilityChangeBound);
document.addEventListener('beforeunload', this.rescueEditorContentBound);
}

private removeEventListeners() {
this.handleWorkPackageUpdate = this.handleWorkPackageUpdate.bind(this);
this.handleVisibilityChange = this.handleVisibilityChange.bind(this);
this.rescueEditorContent = this.rescueEditorContent.bind(this);
document.removeEventListener('work-package-updated', this.handleWorkPackageUpdate);
document.removeEventListener('visibilitychange', this.handleVisibilityChange);
document.removeEventListener('beforeunload', this.rescueEditorContent);
document.removeEventListener('work-package-updated', this.handleWorkPackageUpdateBound);
document.removeEventListener('visibilitychange', this.handleVisibilityChangeBound);
document.removeEventListener('beforeunload', this.rescueEditorContentBound);
}

private handleVisibilityChange() {
Expand Down Expand Up @@ -112,7 +113,6 @@ export default class IndexController extends Controller {
const text = await response.text();
Turbo.renderStreamMessage(text);
this.setLastUpdateTimestamp();
this.hideLastPartOfTimeLineStem();
setTimeout(() => {
if (this.sortingValue === 'asc' && journalsContainerAtBottom) {
// scroll to (new) bottom if sorting is ascending and journals container was already at bottom before a new activity was added
Expand Down Expand Up @@ -395,7 +395,6 @@ export default class IndexController extends Controller {
this.journalsContainerTarget,
this.sortingValue === 'asc',
);
this.hideLastPartOfTimeLineStem();
if (this.isMobile()) {
this.scrollInputContainerIntoView(300);
}
Expand All @@ -419,24 +418,4 @@ export default class IndexController extends Controller {
setLastUpdateTimestamp() {
this.lastUpdateTimestamp = new Date().toISOString();
}

hideLastPartOfTimeLineStem() {
// TODO: I wasn't able to find a pure CSS solution
// Didn't want to identify on server-side which element is last in the list in order to avoid n+1 queries
// happy to get rid of this hacky JS solution!
//
// Note: below works but not if filter is changed, skipping for now
//
// this.element.querySelectorAll('.details-container--empty--last--asc').forEach((container) => container.classList.remove('details-container--empty--last--asc'));

// const containers = this.element.querySelectorAll('.details-container--empty--asc');
// if (containers.length > 0) {
// const lastContainer = containers[containers.length - 1] as HTMLElement;
// // only apply for stem part after comment box
// if (lastContainer?.parentElement?.parentElement?.previousElementSibling?.classList.contains('comment-border-box')) {
// lastContainer.classList.add('details-container--empty--last--asc');
// }
// // lastContainer.classList.add('details-container--empty--last--asc');
// }
}
}
Loading

0 comments on commit 0f276c7

Please sign in to comment.