Skip to content

Commit

Permalink
Add feedback button (#527)
Browse files Browse the repository at this point in the history
Due to a forced push of the dev branch, the following list might
unfortunately include items from other branches as well.

* Init Feedback model

* Add Feedback modal view and corresponding controller

First working version, of course still a lot to improve from here.

* Migrate feedback form to Bootstrap v5

* Add basic styling to Feedback form

* Add "allow contact via mail" checkbox

A new column was added to the Feedbacks schema.
Note that we did not create a new migration as this is a PR which should
only contain one migration, namely the one for the creation of the whol
Feedback table.

* Toggle "allow email contact" by default

* Improve submit button handler (outsource to function)

* Init feedback mailer

Right now just for ourselves, so that we get a plaintext mail with
the feedback of a user.

Env variables were adjusted accordingly, but need to be set manually
in the production environment!

* Adjust feedback mail in views

* Implement success/error flow with toast messages

* Add missing database field "can_contact"

* Add internationalization to feedback error/success

* Lint some files

* Set feedback text field as required with min 10 chars

* Add "optional" to title in email

* Adjust spacing around feedback button

* Internationalize tooltip

* Delete console log

* Add comment describing hidden submit button handler

* Delete default test specs

* Add proper validation for Feedback body

Alongside this, also made sure that we use a custom client-side
validation message when input is too short (under 10 chars long).
This allows us to use the language the user has selected in MaMpf
instead of the browser language.

* Default `can_contact` to false in backend

* Update bootstrap to v5.3.1

command used: bundle update bootstrap
bundle update bootstrap --conservative did not work, as docker
containers did not start again due to dependency errors

* Revert "Update bootstrap to v5.3.1" in favor of PR #537

This reverts commit 5cd1af2.

* Submit form via Ctrl + Enter when modal is opened

* Remove default nil value from ENV.fetch()

* Revert "Remove default nil value from ENV.fetch()"

This reverts commit 696a395.

* Rename button to 'Send' (not 'Save')

* Check if should register feedback event handlers

* Make feedback button ID more specific

* Fix line wrapping (code style)

* Use delete on cascade to be able to delete a user

even if he/she has sent some feedback

* Move Send button before Cancel button

* Replace "on delete cascade" with "dependent destroy"

* Add cypress rules to ESLint & ignore some patterns

* Allow usage of tempusDominus global variable

* Ignore JS files with Sprocket syntax

* Further improve rules, e.g. allow common globals

* Ignore sprocket syntax in cable.js

* Autofix all `.js` and `.js.erb` files

Command used:
`yarn run eslint --fix .`

Still 47 problems (27 errors, 20 warnings) after this.

* Fix variables in turbolink fix

* Prepend unused variables with "_"

* Get rid of unused widget variable

* Fix specs comment tab alignment

* Warn about too long GitHub commit messages (#586)

* Fix comment status (#585)

* Reapply first fix for Reader/Media

See discussion on #574 for further details.
Previous PR for this was #576, closed in favor of this one
as this directly branches off the new "dev" branch.

* Correctly show latest post (might be current_user's comment)

* Fix update of unread comments logic in comments controller

* Fix update icon logic and latest post comment

* Simplify latest comment logic

* Improve code comments

* Further improve comments

* Fix wording in comment

* Fix construction of media array & use `.blank?` instead of `.empty?`

* Migrate `unread_comments` flag (fix inconsistencies) (#587)

* Add dummy migration

* Implement migration for unread comment flag

* Remove unnecessary comment

* Declare migration as not idempotent

* Use array.length instead of counting

* Throw error to prevent revert of migration

* Fix severe flaws in unread comments migration

* Simplify Reader retrieval

* Use the more explicit `.nil?` method

* Update migration date

* Fix annoying bug: don't use `.select!` but `.select`

* Polish migration

e.g. update comment, more suitable name for the method etc.

* Rename method according to #585

* Use `warn` log level for migration (#588)

* Fix linting in feedback.js

* Fix RuboCop errors

* Fix remaining ESLint errors

* Update timestamp of feedback migration

* Add missing Feedback email to prod docker.env

* Remove unnecessary Feedback env variables

* Add validation message for empty body

* Change `const` to `var` to avoid "redefined" errors

* Update timestamp of feedback migration (again)
  • Loading branch information
Splines committed Mar 20, 2024
1 parent 469bb11 commit 4e031a6
Show file tree
Hide file tree
Showing 26 changed files with 376 additions and 4 deletions.
74 changes: 74 additions & 0 deletions app/assets/javascripts/feedback.js
@@ -0,0 +1,74 @@
$(document).on("turbolinks:load", () => {
if (!shouldRegisterFeedback()) {
return;
}
registerToasts();
registerSubmitButtonHandler();
registerFeedbackBodyValidator();
});

var SUBMIT_FEEDBACK_ID = "#submit-feedback";

var TOAST_OPTIONS = {
animation: true,
autohide: true,
delay: 6000, // autohide after ... milliseconds
};

function shouldRegisterFeedback() {
return $(SUBMIT_FEEDBACK_ID).length > 0;
}

function registerToasts() {
const toastElements = document.querySelectorAll(".toast");
[...toastElements].map((toast) => {
new bootstrap.Toast(toast, TOAST_OPTIONS);
});
}

function registerSubmitButtonHandler() {
// Invoke the hidden submit button inside the actual Rails form
$("#submit-feedback-form-btn-outside").click(() => {
submitFeedback();
});

// Submit form by pressing Ctrl + Enter
document.addEventListener("keydown", (event) => {
const isModalOpen = $(SUBMIT_FEEDBACK_ID).is(":visible");
if (isModalOpen && event.ctrlKey && event.key == "Enter") {
submitFeedback();
}
});
}

function registerFeedbackBodyValidator() {
const feedbackBody = document.getElementById("feedback_feedback");
feedbackBody.addEventListener("input", () => {
validateFeedback();
});
}

function validateFeedback() {
const feedbackBody = document.getElementById("feedback_feedback");
const validityState = feedbackBody.validity;
if (validityState.tooShort) {
const tooShortMessage = feedbackBody.dataset.tooShortMessage;
feedbackBody.setCustomValidity(tooShortMessage);
}
else if (validityState.valueMissing) {
const valueMissingMessage = feedbackBody.dataset.valueMissingMessage;
feedbackBody.setCustomValidity(valueMissingMessage);
}
else {
// render input valid, so that form will submit
feedbackBody.setCustomValidity("");
}

feedbackBody.reportValidity();
}

function submitFeedback() {
const submitButton = $("#submit-feedback-form-btn");
validateFeedback();
submitButton.click();
}
2 changes: 2 additions & 0 deletions app/assets/javascripts/lectures.coffee
Expand Up @@ -214,6 +214,7 @@ $(document).on 'turbolinks:load', ->
$('#secondnav').show()
$('#lecturesDropdown').appendTo($('#secondnav'))
$('#notificationDropdown').appendTo($('#secondnav'))
$('#feedback-btn').appendTo($('#secondnav'))
$('#searchField').appendTo($('#secondnav'))
$('#second-admin-nav').show()
$('#adminDetails').appendTo($('#second-admin-nav'))
Expand All @@ -236,6 +237,7 @@ $(document).on 'turbolinks:load', ->
$('#secondnav').hide()
$('#lecturesDropdown').appendTo($('#firstnav'))
$('#notificationDropdown').appendTo($('#firstnav'))
$('#feedback-btn').appendTo($('#firstnav'))
$('#searchField').appendTo($('#firstnav'))
$('#second-admin-nav').hide()
$('#teachableDrop').appendTo($('#first-admin-nav'))
Expand Down
4 changes: 4 additions & 0 deletions app/assets/stylesheets/application.scss
Expand Up @@ -302,4 +302,8 @@ a {
&:hover {
text-decoration: underline;
}
}

.toast {
background-color: white;
}
11 changes: 11 additions & 0 deletions app/assets/stylesheets/feedback.scss
@@ -0,0 +1,11 @@
#feedback-btn {
color: white;

&:focus {
box-shadow: none;
}

&:hover {
color: #ffc107;
}
}
21 changes: 21 additions & 0 deletions app/controllers/feedbacks_controller.rb
@@ -0,0 +1,21 @@
class FeedbacksController < ApplicationController
authorize_resource except: [:create]

def create
feedback = Feedback.new(feedback_params)
feedback.user_id = current_user.id
@feedback_success = feedback.save

if @feedback_success
FeedbackMailer.with(feedback: feedback).new_user_feedback_email.deliver_later
end

respond_to(&:js)
end

private

def feedback_params
params.require(:feedback).permit(:title, :feedback, :can_contact)
end
end
15 changes: 15 additions & 0 deletions app/mailers/feedback_mailer.rb
@@ -0,0 +1,15 @@
class FeedbackMailer < ApplicationMailer
default from: DefaultSetting::FEEDBACK_EMAIL
layout false

# Mail to the MaMpf developers including the new feedback of a user.
def new_user_feedback_email
@feedback = params[:feedback]
reply_to_mail = @feedback.can_contact ? @feedback.user.email : ""
subject = "Feedback: #{@feedback.title}"
mail(to: DefaultSetting::FEEDBACK_EMAIL,
subject: subject,
content_type: "text/plain",
reply_to: reply_to_mail)
end
end
10 changes: 10 additions & 0 deletions app/models/feedback.rb
@@ -0,0 +1,10 @@
# Feedback from users regarding MaMpf itself.
class Feedback < ApplicationRecord
belongs_to :user

BODY_MIN_LENGTH = 10
BODY_MAX_LENGTH = 10_000
validates :feedback, length: { minimum: BODY_MIN_LENGTH,
maximum: BODY_MAX_LENGTH },
allow_blank: false
end
3 changes: 3 additions & 0 deletions app/models/user.rb
Expand Up @@ -78,6 +78,9 @@ class User < ApplicationRecord

# a user has a watchlist with watchlist_entries
has_many :watchlists, dependent: :destroy

has_many :feedbacks, dependent: :destroy

include ScreenshotUploader[:image]

# if a homepage is given it should at leat be a valid address
Expand Down
13 changes: 13 additions & 0 deletions app/views/feedback_mailer/new_user_feedback_email.text.erb
@@ -0,0 +1,13 @@
# Title (optional)
<%= @feedback.title %>

# Feedback
<%= @feedback.feedback %>


-----
<% if @feedback.can_contact %>
Reply to this mail to contact the user.
<% else %>
User did not give permission to contact them regarding their feedback, so we cannot reply to this mail.
<% end %>
44 changes: 44 additions & 0 deletions app/views/feedbacks/_feedback.html.erb
@@ -0,0 +1,44 @@
<%= stylesheet_link_tag 'feedback' %>
<%# Main Modal %>
<div id="submit-feedback" class="modal fade" tabindex="-1" role="dialog" aria-labelledby="Submit feedback" aria-hidden="true">
<div class="modal-dialog modal-lg">
<div class="modal-content p-4">
<%= render partial: 'feedbacks/feedback_form' %>
</div>
</div>
</div>

<%# Messages toasts %>
<div class="toast-container position-fixed bottom-0 end-0 p-3">
<%# Error %>
<div role="alert" aria-live="assertive" aria-atomic="true"
class="toast" id="toast-could-not-send">

<div class="toast-header text-danger">
<i class="bi bi-x-square-fill pe-1"></i>
<strong class="me-auto"> <%= t('feedback.error_short') %></strong>
<button type="button" class="btn-close" data-bs-dismiss="toast" aria-label="Close"></button>
</div>

<div class="toast-body">
<%= t('feedback.error') %>
</div>
</div>

<%# Success %>
<div role="alert" aria-live="assertive" aria-atomic="true"
class="toast" id="toast-successfully-sent">

<div class="toast-header text-success">
<i class="bi bi-check-square-fill pe-1"></i>
<strong class="me-auto"> <%= t('feedback.success_short') %></strong>
<button type="button" class="btn-close" data-bs-dismiss="toast" aria-label="Close"></button>
</div>

<div class="toast-body">
<%= t('feedback.success') %>
</div>
</div>

</div>
8 changes: 8 additions & 0 deletions app/views/feedbacks/_feedback_button.html.erb
@@ -0,0 +1,8 @@
<%= stylesheet_link_tag 'feedback' %>
<%# Feedback button %>
<button type="button" id="feedback-btn" class="btn ms-1 me-1" title="<%= t('feedback.tooltip') %>"
data-bs-toggle="modal"
data-bs-target="#submit-feedback">
<i class="bi bi-star-fill"></i>
</button>
63 changes: 63 additions & 0 deletions app/views/feedbacks/_feedback_form.html.erb
@@ -0,0 +1,63 @@
<%= javascript_include_tag :feedback %>

<div class="modal-header">
<i class="bi bi-star-fill me-3"></i>
<h1 class="modal-title fs-5"><%= t('feedback.modal_title') %></h1>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close">
</button>
</div>

<div class="modal-body">
<%= t('feedback.description_html',
github_mampf: link_to('GitHub', 'https://github.com/MaMpf-HD/mampf/issues', {target: '_blank'}),
feedback_mail: mail_to(DefaultSetting::FEEDBACK_EMAIL, t('basics.mail_noun'))) %>
<%= form_with model: Feedback.new, remote: true do |f| %>
<!-- Title -->
<div class="form-floating mb-3">
<%= f.text_field :title, class: 'form-control', placeholder: '_' %>
<%= f.label :title, t('feedback.title') %>
</div>

<!-- Comment -->
<div class="form-floating mb-3">
<%= f.text_area :feedback,
class: 'form-control',
placeholder: '_',
style: 'height: 200px',
required: '',
minlength: Feedback::BODY_MIN_LENGTH,
maxlength: Feedback::BODY_MAX_LENGTH,
'data-too-short-message': t('feedback.body_too_short_error',
min_length: Feedback::BODY_MIN_LENGTH),
'data-value-missing-message': t('feedback.body_missing_error') %>
<%= f.label :feedback, t('feedback.comment') %>
</div>

<!-- Email contact -->
<div class="form-check">
<%= f.check_box :can_contact,
checked: 'checked',
class: 'form-check-input' %>
<%= f.label :can_contact,
t('feedback.mail_checkbox', user_mail: @current_user.email),
class: 'form-check-label' %>
</div>

<!-- Submit -->
<!-- Dummy submit button; actual submit button is in the modal footer -->
<%= f.submit 'Submit', id: 'submit-feedback-form-btn', style: 'display: none;' %>
<% end %>
</div>

<div class="modal-footer">
<!-- Submit (Send) -->
<button type="button" id="submit-feedback-form-btn-outside" class="btn btn-primary">
<%= t('buttons.send_variant') %>
</button>

<!-- Cancel -->
<button type="button" class="btn btn-secondary" data-bs-dismiss="modal">
<%= t('buttons.cancel') %>
</button>
</div>
7 changes: 7 additions & 0 deletions app/views/feedbacks/create.js.erb
@@ -0,0 +1,7 @@
<% if @feedback_success %>
$("#submit-feedback").modal("hide");
$("#submit-feedback").find("form").trigger("reset"); // clear form
$("#toast-successfully-sent").toast("show");
<% else %>
$("#toast-could-not-send").toast("show");
<% end %>
4 changes: 2 additions & 2 deletions app/views/shared/_dropdown_notifications.html.erb
@@ -1,7 +1,7 @@
<% relevant_notifications = current_user.notifications
.sort_by(&:created_at).reverse %>
<% if relevant_notifications.present? %>
<ul class="navbar-nav me-3"
<ul class="navbar-nav"
id="notificationDropdown">
<li class="nav-item">
<div class="btn-group">
Expand Down Expand Up @@ -49,4 +49,4 @@
</div>
</li>
</ul>
<% end %>
<% end %>
2 changes: 1 addition & 1 deletion app/views/shared/_footer.html.erb
Expand Up @@ -46,7 +46,7 @@
bugs: link_to(t('here'),
'https://github.com/MaMpf-HD/mampf/issues',
target: :_blank),
feedback: mail_to(DefaultSetting::PROJECT_EMAIL, nil),
feedback: mail_to(DefaultSetting::FEEDBACK_EMAIL, nil),
background_photo_credit: link_to('Craig S. Kaplan',
'https://github.com/isohedral/hatviz',
target: :_blank)) %>
Expand Down
7 changes: 7 additions & 0 deletions app/views/shared/_navbar.html.erb
@@ -1,5 +1,8 @@
<% I18n.with_locale(current_user.try(:locale)) do %>
<%= stylesheet_link_tag 'navbar' %>
<%= render partial: 'feedbacks/feedback' %>

<div class="navbars-container">
<nav class="navbar navbar-expand" id="firstnav">
<%= link_to image_tag('MaMpf-Logo.svg',
Expand Down Expand Up @@ -90,7 +93,11 @@
<%= render partial: 'shared/dropdown_lectures',
locals: { lecture: @lecture || current_lecture } %>
</ul>

<%= render partial: 'shared/dropdown_notifications' %>
<%= render partial: 'feedbacks/feedback_button' %>
<%= form_tag(search_index_path,
method: 'get',
class: 'form-inline mt-2 mt-md-0',
Expand Down
1 change: 1 addition & 0 deletions config/initializers/default_setting.rb
Expand Up @@ -2,6 +2,7 @@ class DefaultSetting
ERDBEERE_LINK = ENV.fetch("ERDBEERE_SERVER")
MUESLI_LINK = ENV.fetch("MUESLI_SERVER")
PROJECT_EMAIL = ENV.fetch("PROJECT_EMAIL")
FEEDBACK_EMAIL = ENV.fetch("FEEDBACK_EMAIL")
PROJECT_NOTIFICATION_EMAIL = ENV.fetch("PROJECT_NOTIFICATION_EMAIL")
BLOG_LINK = ENV.fetch("BLOG")
URL_HOST_SHORT = ENV.fetch("URL_HOST_SHORT")
Expand Down

0 comments on commit 4e031a6

Please sign in to comment.