diff --git a/Gemfile b/Gemfile index 9f98d3b0d..c70e1eb17 100644 --- a/Gemfile +++ b/Gemfile @@ -8,6 +8,7 @@ gem "importmap-rails" gem "propshaft" gem "stimulus-rails" gem "turbo-rails" +gem "hotwire_combobox", github: "josefarias/hotwire_combobox", branch: :main # Deployment and drivers gem "bootsnap", require: false diff --git a/Gemfile.lock b/Gemfile.lock index d15178a1e..fccbac836 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,14 @@ +GIT + remote: https://github.com/josefarias/hotwire_combobox.git + revision: 57ee6c9087320fab9383a07fa919fe75e0ea72ba + branch: main + specs: + hotwire_combobox (0.3.2) + platform_agent (>= 1.0.1) + rails (>= 7.0.7.2) + stimulus-rails (>= 1.2) + turbo-rails (>= 1.2) + GEM remote: https://rubygems.org/ specs: @@ -157,6 +168,9 @@ GEM parser (3.3.5.0) ast (~> 2.4.1) racc + platform_agent (1.0.1) + activesupport (>= 5.2.0) + useragent (~> 0.16.3) propshaft (1.1.0) actionpack (>= 7.0.0) activesupport (>= 7.0.0) @@ -319,6 +333,7 @@ DEPENDENCIES brakeman capybara debug + hotwire_combobox! importmap-rails propshaft puma (>= 5.0) diff --git a/app/assets/stylesheets/bubbles.css b/app/assets/stylesheets/bubbles.css index 45d77c069..e327e8150 100644 --- a/app/assets/stylesheets/bubbles.css +++ b/app/assets/stylesheets/bubbles.css @@ -130,11 +130,6 @@ } } - select { - inset: 0; - position: absolute; - } - .avatar { border-radius: var(--bubble-shape); } diff --git a/app/assets/stylesheets/comboboxes.css b/app/assets/stylesheets/comboboxes.css new file mode 100644 index 000000000..5c474ea59 --- /dev/null +++ b/app/assets/stylesheets/comboboxes.css @@ -0,0 +1,20 @@ +:root { + --hw-option-bg-color: var(--color-ink-reversed); + --hw-active-bg-color: var(--color-selected); + --hw-border-color: var(--color-subtle-dark); + --hw-border-radius: 1em; +} + +.hw-combobox { + turbo-frame { + display: inline; + } + + .hw-combobox__main__wrapper { + background-color: var(--color-ink-reversed); + } + + .hw-combobox__input { + background-color: var(--color-ink-reversed); + } +} diff --git a/app/controllers/assignments/toggles_controller.rb b/app/controllers/assignments/toggles_controller.rb new file mode 100644 index 000000000..bcf1616be --- /dev/null +++ b/app/controllers/assignments/toggles_controller.rb @@ -0,0 +1,13 @@ +class Assignments::TogglesController < ApplicationController + include BubbleScoped, BucketScoped + + def create + @bubble.toggle_assignment assignee + redirect_to @bubble + end + + private + def assignee + @bucket.users.active.find params.expect(:assignee_id) + end +end diff --git a/app/controllers/assignments_controller.rb b/app/controllers/assignments_controller.rb deleted file mode 100644 index 872725b53..000000000 --- a/app/controllers/assignments_controller.rb +++ /dev/null @@ -1,19 +0,0 @@ -class AssignmentsController < ApplicationController - include BubbleScoped, BucketScoped - - def new - end - - def show - end - - def create - @bubble.assign(find_assignee) - redirect_to @bubble - end - - private - def find_assignee - @bucket.users.active.find(params.expect(:assignee_id)) - end -end diff --git a/app/controllers/taggings/toggles_controller.rb b/app/controllers/taggings/toggles_controller.rb new file mode 100644 index 000000000..f1b611eb7 --- /dev/null +++ b/app/controllers/taggings/toggles_controller.rb @@ -0,0 +1,17 @@ +class Taggings::TogglesController < ApplicationController + include BubbleScoped, BucketScoped + + def create + @bubble.toggle_tag tag + redirect_to @bubble + end + + private + def tag + if params[:tag_title] + Tag.new title: params[:tag_title] + else + Current.account.tags.find params.expect(:tag_id) + end + end +end diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb deleted file mode 100644 index 275ffce05..000000000 --- a/app/controllers/tags_controller.rb +++ /dev/null @@ -1,23 +0,0 @@ -class TagsController < ApplicationController - include BucketScoped - - before_action :set_bubble, only: %i[ new create ] - - def index - @tags = Current.account.tags.order(:title) - end - - def new - end - - # FIXME: Should move this to a taggings controller to separate tag administration from applying - def create - @bubble.tag(params.require(:tag).expect(:title)) - redirect_to @bubble - end - - private - def set_bubble - @bubble = @bucket.bubbles.find(params[:bubble_id]) - end -end diff --git a/app/helpers/combobox_helper.rb b/app/helpers/combobox_helper.rb new file mode 100644 index 000000000..c9bb10c10 --- /dev/null +++ b/app/helpers/combobox_helper.rb @@ -0,0 +1,10 @@ +module ComboboxHelper + # `hw_combobox_tag` overrides HotwireCombobox's helper to provide default options. + def hw_combobox_tag(...) + super do |combobox| + combobox.customize_main_wrapper class: "btn" + combobox.customize_input data: { autofocus_target: "element" } + yield combobox if block_given? + end + end +end diff --git a/app/javascript/controllers/autofocus_controller.js b/app/javascript/controllers/autofocus_controller.js new file mode 100644 index 000000000..75e5c4de9 --- /dev/null +++ b/app/javascript/controllers/autofocus_controller.js @@ -0,0 +1,16 @@ +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static targets = [ "element" ] + + connect() { + this.observer = new IntersectionObserver(([entry]) => { + if (entry.isIntersecting) this.elementTarget.focus() + }) + this.observer.observe(this.elementTarget) + } + + disconnect() { + this.observer.disconnect() + } +} diff --git a/app/javascript/controllers/expandable_controller.js b/app/javascript/controllers/expandable_controller.js new file mode 100644 index 000000000..74b3bc050 --- /dev/null +++ b/app/javascript/controllers/expandable_controller.js @@ -0,0 +1,9 @@ +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + closeOnFocusOutside(event) { + if (!this.element.contains(event.relatedTarget)) { + this.element.removeAttribute("open") + } + } +} diff --git a/app/models/assignment.rb b/app/models/assignment.rb index 54abe682e..7233030b3 100644 --- a/app/models/assignment.rb +++ b/app/models/assignment.rb @@ -3,6 +3,4 @@ class Assignment < ApplicationRecord belongs_to :assignee, class_name: "User" belongs_to :assigner, class_name: "User" - - validates :assignee, uniqueness: { scope: :bubble } end diff --git a/app/models/bubble/assignable.rb b/app/models/bubble/assignable.rb index 0b6dc3f9a..1e6d0bf56 100644 --- a/app/models/bubble/assignable.rb +++ b/app/models/bubble/assignable.rb @@ -10,12 +10,24 @@ module Bubble::Assignable scope :assigned_by, ->(users) { joins(:assignments).where(assignments: { assigner: users }).distinct } end - def assign(users, assigner: Current.user) - assignee_rows = Array(users).collect { |user| { assignee_id: user.id, assigner_id: assigner.id, bubble_id: id } } - - transaction do - Assignment.insert_all assignee_rows - track_event :assigned, assignee_ids: assignee_rows.pluck(:assignee_id) - end + def toggle_assignment(user) + assigned_to?(user) ? unassign(user) : assign(user) end + + def assigned_to?(user) + assignments.exists? assignee: user + end + + private + def assign(user) + assignments.create! assignee: user, assigner: Current.user + track_event :assigned, assignee_ids: [ user.id ] + rescue ActiveRecord::RecordNotUnique + # Already assigned + end + + def unassign(user) + destructions = assignments.destroy_by assignee: user + track_event :unassigned, assignee_ids: [ user.id ] if destructions.any? + end end diff --git a/app/models/bubble/taggable.rb b/app/models/bubble/taggable.rb index 90bef0eb2..6b6462e9b 100644 --- a/app/models/bubble/taggable.rb +++ b/app/models/bubble/taggable.rb @@ -8,7 +8,22 @@ module Bubble::Taggable scope :tagged_with, ->(tags) { joins(:taggings).where(taggings: { tag: tags }) } end - def tag(title) - taggings.create! tag: bucket.account.tags.find_or_create_by!(title: title) + def toggle_tag(tag) + tagged_with?(tag) ? untag(tag) : tag(tag) end + + def tagged_with?(tag) + tags.include? tag + end + + private + def tag(tag) + taggings.create! tag: tag + rescue ActiveRecord::RecordNotUnique + # Already tagged + end + + def untag(tag) + taggings.destroy_by tag: tag + end end diff --git a/app/models/event_summary.rb b/app/models/event_summary.rb index de90ff447..2c1d23b07 100644 --- a/app/models/event_summary.rb +++ b/app/models/event_summary.rb @@ -21,6 +21,8 @@ class EventSummary < ApplicationRecord "Added by #{event.creator.name} #{time_ago_in_words(event.created_at)} ago." when "assigned" "Assigned to #{event.assignees.pluck(:name).to_sentence} #{time_ago_in_words(event.created_at)} ago." + when "unassigned" + "Unassigned from #{event.assignees.pluck(:name).to_sentence} #{time_ago_in_words(event.created_at)} ago." when "staged" "#{event.creator.name} moved this to '#{event.stage_name}'." when "unstaged" diff --git a/app/models/tag.rb b/app/models/tag.rb index c4cd4209e..e11391e0b 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -1,7 +1,7 @@ class Tag < ApplicationRecord include Filterable - belongs_to :account + belongs_to :account, default: -> { Current.account } has_many :taggings, dependent: :destroy has_many :bubbles, through: :taggings @@ -9,4 +9,8 @@ class Tag < ApplicationRecord def hashtag "#" + title end + + def to_combobox_display + title + end end diff --git a/app/models/user.rb b/app/models/user.rb index d452c1756..2baa83876 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -40,6 +40,10 @@ class User < ApplicationRecord other != self end + def to_combobox_display + name + end + private def deactived_email_address email_address.sub(/@/, "-deactivated-#{SecureRandom.uuid}@") diff --git a/app/views/bubbles/_assignments.html.erb b/app/views/bubbles/_assignments.html.erb index d1bec1073..7206ef4f5 100644 --- a/app/views/bubbles/_assignments.html.erb +++ b/app/views/bubbles/_assignments.html.erb @@ -1,15 +1,9 @@ -<% bubble.assignments.each do |assignment| %> -
- <%= form_with url: bucket_bubble_assignments_path(bubble.bucket, bubble), data: { controller: "form" } do |form| %> - - <% end %> -
+<% bubble.assignees.each do |assignee| %> +
+
+ <%= assignee.name %> + <%= avatar_image_tag assignee, loading: :lazy, class: "avatar flex-item-no-shrink" %> + Assigned to <%= assignee.name %> +
+
<% end %> diff --git a/app/views/bubbles/show.html.erb b/app/views/bubbles/show.html.erb index 326eab71a..959f5093b 100644 --- a/app/views/bubbles/show.html.erb +++ b/app/views/bubbles/show.html.erb @@ -23,44 +23,8 @@ <% end %> - <%= form_with url: bucket_bubble_assignments_path(@bubble.bucket, @bubble), data: { controller: "form" } do |form| %> - - <% end %> - -
- - - - <%= image_tag "tag.svg", aria: { hidden: true }, size: 24 %> - - <%= form_with model: Tag.new, url: bucket_bubble_tags_path(@bubble.bucket, @bubble), class: "min-width flex-item-grow", data: { turbo_frame: "_top" } do |form| %> - <%= form.text_field :title, class: "input", autofocus: "on", list: "tags-list" %> - <%= form.submit "Create Tag", hidden: true %> - - - <%= Current.account.tags.each do |tag| %> - - <% end %> - - <% end %> - -
- -
- -
+ <%= render "bubbles/sidebar/assignment", bubble: @bubble, users: @bucket.users.active %> + <%= render "bubbles/sidebar/tag", bubble: @bubble, tags: Current.account.tags %> <% if @bubble.image.attached? %> <%= button_to bucket_bubble_image_path(@bubble.bucket, @bubble), method: :delete, class: "btn full-width justify-start borderless" do %> diff --git a/app/views/bubbles/sidebar/_assignment.html.erb b/app/views/bubbles/sidebar/_assignment.html.erb new file mode 100644 index 000000000..2236cb639 --- /dev/null +++ b/app/views/bubbles/sidebar/_assignment.html.erb @@ -0,0 +1,12 @@ +
+ + <%= image_tag "person.svg", aria: { hidden: true }, size: 24 %> + Assign to someone + + + <%= form_with url: bucket_bubble_assignment_toggles_path(bubble.bucket, bubble), data: { controller: "form" } do |form| %> + <%= form.combobox :assignee_id, users, required: true, + render_in: { partial: "bubbles/users/listbox_option", as: :user, locals: { bubble: bubble } }, + data: { controller: "autofocus", action: "hw-combobox:selection->form#submit" } %> + <% end %> +
diff --git a/app/views/bubbles/sidebar/_tag.html.erb b/app/views/bubbles/sidebar/_tag.html.erb new file mode 100644 index 000000000..c367be378 --- /dev/null +++ b/app/views/bubbles/sidebar/_tag.html.erb @@ -0,0 +1,12 @@ +
+ + <%= image_tag "tag.svg", aria: { hidden: true }, size: 24 %> + Tag this + + + <%= form_with url: bucket_bubble_tagging_toggles_path(bubble.bucket, bubble), data: { controller: "form" } do |form| %> + <%= form.combobox :tag_id, tags, required: true, name_when_new: "tag_title", + render_in: { partial: "bubbles/tags/listbox_option", as: :tag, locals: { bubble: bubble } }, + data: { controller: "autofocus", action: "hw-combobox:selection->form#submit" } %> + <% end %> +
diff --git a/app/views/bubbles/tags/_listbox_option.html.erb b/app/views/bubbles/tags/_listbox_option.html.erb new file mode 100644 index 000000000..b1176ddc0 --- /dev/null +++ b/app/views/bubbles/tags/_listbox_option.html.erb @@ -0,0 +1,7 @@ +
+ <%= tag.hashtag %> + + <% if bubble.tagged_with?(tag) %> + <%= image_tag "check.svg", aria: { hidden: "true" } %> + <% end %> +
diff --git a/app/views/bubbles/users/_listbox_option.html.erb b/app/views/bubbles/users/_listbox_option.html.erb new file mode 100644 index 000000000..ec304a714 --- /dev/null +++ b/app/views/bubbles/users/_listbox_option.html.erb @@ -0,0 +1,7 @@ +
+ <%= user.name %> + + <% if bubble.assigned_to?(user) %> + <%= image_tag "check.svg", aria: { hidden: "true" } %> + <% end %> +
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 0534b470b..d977d6af0 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -12,6 +12,7 @@ <%= csp_meta_tag %> <%= tag.meta name: "current-user-id", content: Current.user.id if Current.user %> + <%= combobox_style_tag %> <%= stylesheet_link_tag :app, "data-turbo-track": "reload" %> <%= javascript_importmap_tags %> diff --git a/config/routes.rb b/config/routes.rb index 0778fb130..c11aff6ee 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -16,10 +16,8 @@ Rails.application.routes.draw do resources :buckets do resources :bubbles do - resources :assignments resources :boosts resources :comments - resources :tags, shallow: true scope module: :bubbles do resource :image @@ -27,9 +25,15 @@ Rails.application.routes.draw do resource :stage_picker resources :stagings end - end - resources :tags, only: :index + namespace :assignments, as: :assignment do + resources :toggles + end + + namespace :taggings, as: :tagging do + resources :toggles + end + end end resources :filters diff --git a/db/migrate/20241120222444_make_taggings_unique_per_bubble.rb b/db/migrate/20241120222444_make_taggings_unique_per_bubble.rb new file mode 100644 index 000000000..06edb6170 --- /dev/null +++ b/db/migrate/20241120222444_make_taggings_unique_per_bubble.rb @@ -0,0 +1,6 @@ +class MakeTaggingsUniquePerBubble < ActiveRecord::Migration[8.0] + def change + remove_index :taggings, :bubble_id + add_index :taggings, %i[ bubble_id tag_id ], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 9754ac707..9895b7729 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2024_11_19_201943) do +ActiveRecord::Schema[8.0].define(version: 2024_11_20_222444) do create_table "accesses", force: :cascade do |t| t.integer "bucket_id", null: false t.integer "user_id", null: false @@ -187,7 +187,7 @@ ActiveRecord::Schema[8.0].define(version: 2024_11_19_201943) do t.integer "tag_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.index ["bubble_id"], name: "index_taggings_on_bubble_id" + t.index ["bubble_id", "tag_id"], name: "index_taggings_on_bubble_id_and_tag_id", unique: true t.index ["tag_id"], name: "index_taggings_on_tag_id" end diff --git a/test/controllers/assignments/toggles_controller_test.rb b/test/controllers/assignments/toggles_controller_test.rb new file mode 100644 index 000000000..b82e18099 --- /dev/null +++ b/test/controllers/assignments/toggles_controller_test.rb @@ -0,0 +1,19 @@ +require "test_helper" + +class Assignments::TogglesControllerTest < ActionDispatch::IntegrationTest + setup do + sign_in_as :kevin + end + + test "create" do + assert_changes "bubbles(:logo).assigned_to?(users(:david))", from: false, to: true do + post bucket_bubble_assignment_toggles_url(buckets(:writebook), bubbles(:logo)), params: { assignee_id: users(:david).id } + end + assert_redirected_to bubbles(:logo) + + assert_changes "bubbles(:logo).assigned_to?(users(:david))", from: true, to: false do + post bucket_bubble_assignment_toggles_url(buckets(:writebook), bubbles(:logo)), params: { assignee_id: users(:david).id } + end + assert_redirected_to bubbles(:logo) + end +end diff --git a/test/controllers/taggings/toggles_controller_test.rb b/test/controllers/taggings/toggles_controller_test.rb new file mode 100644 index 000000000..fe9e76a8e --- /dev/null +++ b/test/controllers/taggings/toggles_controller_test.rb @@ -0,0 +1,19 @@ +require "test_helper" + +class Taggings::TogglesControllerTest < ActionDispatch::IntegrationTest + setup do + sign_in_as :kevin + end + + test "create" do + assert_changes "bubbles(:logo).tagged_with?(tags(:mobile))", from: false, to: true do + post bucket_bubble_tagging_toggles_url(buckets(:writebook), bubbles(:logo)), params: { tag_id: tags(:mobile).id } + end + assert_redirected_to bubbles(:logo) + + assert_changes "bubbles(:logo).tagged_with?(tags(:mobile))", from: true, to: false do + post bucket_bubble_tagging_toggles_url(buckets(:writebook), bubbles(:logo)), params: { tag_id: tags(:mobile).id } + end + assert_redirected_to bubbles(:logo) + end +end diff --git a/test/controllers/tags_controller_test.rb b/test/controllers/tags_controller_test.rb deleted file mode 100644 index 909b806e2..000000000 --- a/test/controllers/tags_controller_test.rb +++ /dev/null @@ -1,24 +0,0 @@ -require "test_helper" - -class TagsControllerTest < ActionDispatch::IntegrationTest - setup do - sign_in_as :kevin - end - - test "create with existing tag" do - assert_no_difference -> { accounts(:"37s").tags.count } do - post bucket_bubble_tags_url(buckets(:writebook), bubbles(:logo)), params: { tag: { title: "Web" } } - end - - assert_redirected_to bubbles(:logo) - end - - test "create with new tag" do - assert_difference -> { accounts(:"37s").tags.count }, +1 do - post bucket_bubble_tags_url(buckets(:writebook), bubbles(:logo)), params: { tag: { title: "Horizons" } } - end - - assert_redirected_to bubbles(:logo) - assert bubbles(:logo).tags.pluck(:title).include?("Horizons") - end -end diff --git a/test/models/bubble_test.rb b/test/models/bubble_test.rb index 40cdbe4ad..d32ec380c 100644 --- a/test/models/bubble_test.rb +++ b/test/models/bubble_test.rb @@ -19,11 +19,51 @@ class BubbleTest < ActiveSupport::TestCase end end - test "assigning" do - bubbles(:logo).assign users(:david) + test "assignment states" do + assert bubbles(:logo).assigned_to?(users(:kevin)) + assert_not bubbles(:logo).assigned_to?(users(:david)) + end - assert_equal users(:kevin, :jz, :david), bubbles(:logo).assignees - assert_equal [ users(:david) ], Event.last.assignees + test "assignment toggling" do + assert bubbles(:logo).assigned_to?(users(:kevin)) + + assert_difference({ "bubbles(:logo).assignees.count" => -1, "Event.count" => +1 }) do + bubbles(:logo).toggle_assignment users(:kevin) + end + assert_not bubbles(:logo).assigned_to?(users(:kevin)) + assert_equal "unassigned", Event.last.action + assert_equal [ users(:kevin) ], Event.last.assignees + + assert_difference %w[ bubbles(:logo).assignees.count Event.count ], +1 do + bubbles(:logo).toggle_assignment users(:kevin) + end + assert bubbles(:logo).assigned_to?(users(:kevin)) + assert_equal "assigned", Event.last.action + assert_equal [ users(:kevin) ], Event.last.assignees + end + + test "tagged states" do + assert bubbles(:logo).tagged_with?(tags(:web)) + assert_not bubbles(:logo).tagged_with?(tags(:mobile)) + end + + test "tag toggling" do + assert bubbles(:logo).tagged_with?(tags(:web)) + + assert_difference "bubbles(:logo).taggings.count", -1 do + bubbles(:logo).toggle_tag tags(:web) + end + assert_not bubbles(:logo).tagged_with?(tags(:web)) + + assert_difference "bubbles(:logo).taggings.count", +1 do + bubbles(:logo).toggle_tag tags(:web) + end + assert bubbles(:logo).tagged_with?(tags(:web)) + + assert_difference %w[ bubbles(:logo).taggings.count accounts("37s").tags.count ], +1 do + bubbles(:logo).toggle_tag Tag.new(title: "prioritized") + end + assert_equal "prioritized", bubbles(:logo).taggings.last.tag.title end test "searchable by title" do