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

Contact Form Feedback as Modal Issue#357 #422

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

spark-c
Copy link
Contributor

@spark-c spark-c commented Sep 14, 2021

Hello!

This PR is an in-progress implementation of Issue #357.

I think my implementation may be a little hacky, and I'm definitely open to an alternative solution! (I wanted to give it a go alone first to see if I could do it 😃 ). I'll explain my approach below.

P.S. I write long... my bad. I went into detail b/c I wanted to make sure I understood / to talk myself through everything once more.

Before the wall of text below, here are my questions:

  • How do you feel about the implementation?

  • Where should the form and/or modal redirect?


Demo:

chipy.contact.mp4

What was the previous functionality?

Previously, submitting the contact form resulted in a redirect to the chipy.org homepage, where a success/fail message would be presented like so:

previously

Redirect

In its current state, this PR updates chipy_org/apps/contact/views.py such that the form submission redirects to the /contact page instead of going back to the homepage. It felt a little surprising to me when I was taken back to home by the "Send" button without warning.

If you'd like to preserve the old behavior, I think it might feel good if:

  1. The "Send" button submits the form and stays on /contact page
  2. The modal appears showing feedback
  3. The button to close the modal reads "Return to Home" and redirects accordingly

How was the modal HTML built?

I found an existing modal implementation in use for the site's login functionality; the HTML is located in shiny/navbar.html, and the javascript for its behavior is found in static/js/pinax.modal.js (neither file has been modified).

Here is where I begin to get concerned about being a bit hacky -- I copy/pasted the HTML for the Login Modal into the apps/contact/templates/contact.html file, and made appropriate adjustments to its structure/data and classes.

How was the modal behavior built?

I continued to use the django.contrib.messages framework that already existed:

  • I removed the behavior in shiny/slim.html that displayed the original green/red box w/message
  • {% include "_messages.html" %} is now accessed in contact.html, inside the modal body
  • in _messages.html, the messages object is still looped through in an identical way as before, except without the logic that styles the box red/green depending on success/failure.
    • The "X" close button in messages.html file was removed, in favor of replacing it with the "Return to Site" button which now lives in contact.html as part of the modal structure.

What triggers the modal to show?

In the original code (pinax.modal.js), the modal is set up to be triggered by clicking an element (adding bootstrap classes to an element that makes it trigger the modal functionality).

Since this feedback should be automatic and not the result of user clicks, I had a little workaround:

  • Added an empty div with appropriate data classes and display:none hardcoded into the tag
  • When user visits the page:
    • (jinja templating) If there is a message to show:
      • add modal HTML
      • (script tag) when the page finishes loading, .click() that empty div to trigger the modal.

@raymondberg
Copy link
Member

Won't have time to look at the code for a bit, but I wanted to call out that this is quite a wonderful PR (description and demo). A great model for anyone opening PRs...on any project.

@spark-c spark-c closed this Sep 16, 2021
@spark-c spark-c reopened this Sep 16, 2021
@spark-c
Copy link
Contributor Author

spark-c commented Sep 16, 2021

Ummm please disregard the "close" and "reopen" that just happened...

I accidentally opened this page with the mobile app, and definitely don't know how to use the mobile app properly. Whoops!

@spark-c
Copy link
Contributor Author

spark-c commented Sep 17, 2021

After meeting yesterday, here are some notes that will be addressed:

  • Submitting the contact form will display the modal on the Contact page, and then the modal button will take the user back to the homepage.
  • Oops, I broke slim.html by removing the existing message boxes!
    • This change was actually unnecessary anyway. Current changes to slim.html will be reverted. May need to make a couple of different adjustments here; see below.
  • I think I'll need to add some conditional logic to _messages.html to decide whether to show the modal or the normal green msgbox.
    • Even so, there are 2-3 divs that are drawn from slim.html that may get in the way. TBD
  • Implement some kind of handling for failure cases when submitting contact form. On failure:
    • ensure the correct failure message is displayed
    • modal button should not redirect to homepage
  • Will ruminate on whether it would be possible to abstract modal behavior into its own template.
  • Also may make a small update to the Captcha on Contact page to use a stronger version to combat current spam issue
    • This could live in this PR, or its own

EDIT:

  • I'm currently working with a little bit of conditional logic and a new piece of config for ContactView. Leaning towards something that might need to be covered by a test.

@spark-c
Copy link
Contributor Author

spark-c commented Sep 20, 2021

Progress Update

I've addressed bullet_points[0:3] above; I created a new little branch and opened an explanatory PR against my own fork of the repo here because I don't want to clutter up this PR's commit history (as I expect to make further improvements).

TBH I'm new to PR commit etiquette so I'm just trying not to make a mess while I'm tinkering around 😆

TL;DR - I fixed what I broke in /templates/slim.html, and the message-box + modal behaviors are now living together in peace. I did this by adding a message_as_modal = True flag in the ContactView, sharing that variable with the template, and letting the template follow the correct behavior according to that flag.

I am working on another branch where the modal functionality is more self-contained instead of copy/pasted, and should be reusable in any view where it may be desirable. No writeup/PR yet, however the work is visible on my fork on the "modularize-modal" branch.

@raymondberg
Copy link
Member

You are fine to update this PR anytime you want, no worries there. But do whatever your workflow requires. I added the label for in progress. Remove that when you are all done, and feel free to ping folks too!

@spark-c
Copy link
Contributor Author

spark-c commented Oct 4, 2021

Hello again! My bad for such a delay.

I think I'm just about done; the modal behavior is fairly reusable now (and no longer breaks any existing behavior 😇).

Just as I was about to walk though and write out the updates, I somehow managed to break my local dev environment...? I'm not exactly sure what happened, but my docker containers are starting with different names than before and I'm getting Django errors saying that certain data doesn't exist (I think both session data and db stuff).

Anyway, I'll be back with once I figure out what's going on. I'm 99.99% sure it's not a code problem, b/c even previously-stable branches are having this issue.

Cheers!

@spark-c
Copy link
Contributor Author

spark-c commented Oct 6, 2021

Okay! Back in business. I just needed to reset the dev-db and remake the data.

At this point in time, I think I'm just about done (P.S. dangit I might need to adjust the img and its css); I can make any changes you like, but I think this has potential to be the PR's final state.


TL;DR:

  • The modal behavior does not break "slim.html" and the currently existing green messagebox is unaffected.
  • The modal behavior can be reused in any view that creates Django messages with the following steps:
    1. Add a flag in the desired View class
    2. Add two pieces of config in a dict to that class (where you want the close button to go, and the button text)
    3. Make sure to pass those variables to the template via the View's get_context_data() method.
  • The modal uses the img/chipy_logo.png image. TBH I should probably change that because it's a little small, and the chipmunk.png is too big. I should probably scale down the large image and use that.
  • Linter is happy 10/10, Tests passing (not sure if any additional tests will be required), Black is happy with all files I've changed (it wants to remove two empty lines in an unrelated file in the Jobs app)

PR update


"omg I love reading complete changelogs":

Here is what has changed vs chipy.org/main:

chipy_org/apps/main/templates/main/homepage.html AND slim.html

These file changes are identical, because every page extends slim except for the homepage; so this code lives in both spots.

Previously:

  • If there was a message to display, the template would render a few divs as containers for the message, and then go to "_messages.html" for the rest of the contents of the box, including the close button and the message itself.
{% if messages %}
<div class="container-xl">
    <div class="row">
        <div class="col">
            {% include "_messages.html" %}
        </div>
    </div>
</div>
{% endif %}

Now:

  • If there is a message to display, check if the View is set to display messages as a modal.
    • If so, use the "_modal.html" template.
    • If not, use the "_messages.html" template. (I moved the three container divs to live with the rest of the code in _messages)
{% if messages %}
    {% if message_as_modal %}
        {% include "_modal.html" %}
    {% else %}
        {% include "_messages.html" %}
    {% endif %}
{% endif %}

chipy_org/apps/contact/views.py

There are two changes of interest to this file, and they are related to:

  • config
  • passing that config to the template

(This is how you tell any view to use the modal behavior!)

Config

  1. In the desired View (in this case, ContactView), add the line message_as_modal = True. This tells the associated templates to use _modal.html to show messages.
  2. To make the modal behavior reusable, I added a small dict of config for fields you may like to change. It looks like this:
modal_config = {
    "close_button_redirect": "/",
    "close_button_label": "Return to home"
}

The first key represents the page where it will redirect the user upon clicking the button to close the modal. The second key is the text that should be used to label the close button.

Passing that config from the View to the Template

As with any variables to be used in the template, we'll use Django context (a dict). The Contact view wasn't already sharing any data with the templates, so I added the method to the view. In some other views, the method is already present; you'll just need to context.update({"foo": "bar"}) with any new data you'd like to pass.

The method looks like this:

    def get_context_data(self, **kwargs):
        """ Used to access message_as_modal in template as context """
        context = super(ContactView, self).get_context_data(**kwargs)
        context.update({"message_as_modal": self.message_as_modal})
        context.update({"modal_config": self.modal_config})
        return context

Now within any templates, we can access this data as {{ message_as_modal }} and {{ modal_config.my_config_key }} respectively.

chipy_org/templates/_modal.html

It may seem that we are using either "_messages.html" or "_modal.html", but this is not the case. In short, "_modal.html" is just a pretty wrapper for the "_messages.html" template within it. "_messages.html" is the only file that handles/empties the message queue.

"_modal.html" has three parts:

  1. A single "target" div at the top, which has bootstrap data-classes to designate it for use by "pinax.modal.js" (where the modal open/close/etc functionality comes from). When this div is "clicked", the modal opens. However, it has a display:none so that the user cannot click it manually.
  2. The HTML structure of the modal. This includes everything the user sees in the modal, including the imported "_messages.html" template.
  3. A small script at the bottom. "pinax.modal.js" is built expecting a user to click something to initiate the modal appearing. Since the Django message framework fires based upon page refresh and not user clicks, there needed to be some intermediate solution. The flow works like this:
    1. Page renders
    2. If there is a message and we're using the modal behavior, the script adds a page "load" event listener with callback:
    3. When the page "load" event is triggered, it .click()s the hidden div that opens the modal!

chipy_org/templates/_messages.html

This file serves the message we've all been waiting for! It has one simple conditional:

  • If the config says we're using a modal
    • just return a div with the message in it.
  • If we're not using a modal
    • return all of the stuff that we normally use for the message box.

chipy_org/static/css/shiny.css

Added a few styling rules to make the modal elements align correctly.

@elmq0022
Copy link
Contributor

@spark-c this looks really great. I will review this at the upcoming project night Thursday, November 18th. Thanks for your efforts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants