#245 Improve side navigation design
Merged 5 years ago by jflory7. Opened 5 years ago by alishapapun.
fedora-commops/ alishapapun/fedora-happiness-packets navigation-test  into  master

file modified
+15 -4
@@ -60,10 +60,10 @@ 

      cursor: pointer;

  }

  

- aside .sidebar .total-packets {

+ .total-packets {

      cursor: default;

  }

- aside .sidebar .total-packets:hover{

+ .total-packets:hover{

    text-decoration: none;

  }

  
@@ -385,9 +385,9 @@ 

    text-align: center;

    font-size:1.4rem;

  }

- #search_fasid {

+ .search {

      background-color: white;

-     padding: 0 1em;

+     padding: 0.5rem 1em;

      color: var(--secondary-color);

      font-weight: 500;

      cursor: pointer;
@@ -408,3 +408,14 @@ 

  .fedmenu-content {

      position: absolute !important;

  }

+ .total_happiness_sent{

+     text-align: center;

+     font-size: 3rem;

+     padding: 2rem;

+     font-family: 'Montserrat';

+ }

+ #id_q {

+     height: 3.2rem;

+     padding: 0 0.8rem;

+     border-radius: 2rem;

+ }

@@ -36,6 +36,7 @@ 

  class MessageSearchView(SearchView):

      template_name = 'search/search.html'

      form_class = SearchForm

+     paginate_by = 5

  

  class ArchiveListView(ListView):

      model = Message

file modified
+5 -12
@@ -65,23 +65,16 @@ 

                          </button>

                      </form>

                  {% endif %}

-                 {% if packets_sent == 0 %}

-                     <li><a class="total-packets">Total Happiness Spread: <b>{{ packets_sent }} 💔</b></a></li>

-                 {% else %}

-                     <li><a class="total-packets">Total Happiness Spread: <b>{{ packets_sent }} 🎉</b></a></li>

-                 {% endif %}

-                 {% url 'messaging:send' as url %}

-                 <li role="presentation" {% if url in request.path %}class="active"{% endif %}><a href="{{ url }}">Send some happiness!</a></li>

                  {% url 'messaging:start' as url %}

-                 <li role="presentation" {% ifequal request.path url %}class="active"{% endifequal %}><a href="{{ url }}">What are Happiness Packets?</a></li>

+                 <li role="presentation" {% ifequal request.path url %}class="active"{% endifequal %}><a href="{{ url }}">Home</a></li>

+                 {% url 'messaging:send' as url %}

+                 <li role="presentation" {% if url in request.path %}class="active"{% endif %}><a href="{{ url }}">Send Happiness</a></li>

+                 {% url 'messaging:archive' as url %}

+                 <li role="presentation" {% ifequal request.path url %}class="active"{% endifequal %}><a href="{{ url }}">Happiness Archive</a></li>

                  {% url 'messaging:inspiration' as url %}

                  <li role="presentation" {% ifequal request.path url %}class="active"{% endifequal %}><a href="{{ url }}">Get inspired</a></li>

                  {% url 'messaging:faq' as url %}

                  <li role="presentation" {% ifequal request.path url %}class="active"{% endifequal %}><a href="{{ url }}">FAQ</a></li>

-                 {% url 'messaging:archive' as url %}

-                 <li role="presentation" {% ifequal request.path url %}class="active"{% endifequal %}><a href="{{ url }}">Happiness Archive</a></li>

-                 {% url 'messaging:search' as url %}

-                 <li role="presentation" {% if url in request.path %}class="active"{% endif %}><a href="{{ url }}">Search the Archive</a></li>

                </ul>

          </div>

      </aside>

@@ -8,6 +8,14 @@ 

          <h2>Happiness Archive</h2>

      </div>

  

+     {% url 'messaging:search' as url %}

+     <form method="get" class="form-inline" action="{{url}}">

+             <div class="form-group" style="font-size: 1.7em;">

+                 <input type="search" name="q" id="id_q">

+                 <input type="submit" value="Search" class="search">

+             </div>  

+     </form>

+ 

      <p>

          When you send a Happiness Packet, you can agree to display your message publicly on

          this website. If both sender and recipient select this option, the message is published

@@ -15,7 +15,7 @@ 

  </div>

  {% crispy form %}

  

- <button id="search_fasid">Search</button>

+ <button id="search_fasid" class="search">Search</button>

Could id="search_fasid" be removed since it was removed from custom.css?

  

  <script> 

  $("#submit-id-submit").click(function() {

@@ -43,7 +43,15 @@ 

          If both the sender and the recipient agree, we can

          <a href="{% url 'messaging:archive' %}">publish the Happiness Packet</a> on the website.

          We hope to build an archive of open-source happiness that communities can draw inspiration from.

-     </p><p>

+     </p>

+     <div class="total_happiness_sent">

+         {% if packets_sent == 0 %}

+         <a class="total-packets">Total Happiness Spread: <b>{{ packets_sent }} 💔</b></a>

+         {% else %}

+         <a class="total-packets">Total Happiness Spread: <b>{{ packets_sent }} 🎉</b></a>

+         {% endif %}

+     </div>

+     <p>

          As an example, here are two random messages from our archive:

      </p>

  

file modified
+1 -1
@@ -11,7 +11,7 @@ 

          {{ field.label_tag }} {{ field }}

      </div>

      {% endfor %}

-     <input type="submit" value="Search">

+     <input type="submit" value="Search" class="search">

  </form>

  {% if query %}

  <h3>Results</h3>

This PR addresses #184 and includes search functionality in the archive by merging archive page and search page.

Metadata Update from @jflory7:
- Pull-request tagged with: PASSED, improvement, needs testing, type - backend, type - frontend, type - summer coding

5 years ago

Metadata Update from @jflory7:
- Request assigned

5 years ago

Could id="search_fasid" be removed since it was removed from custom.css?

Maybe we could put this above the _message_list so it is easier to see. What do you think?

Could you please choose a more a descriptive name for this CSS ID? When I read through the CSS file, it's not obvious #id_q is for the archive search bar. It should be clear what the style elements are for from the name.

Since this is the landing page and the first page the user will visit when opening the site, it might make more sense to move it to the top of the sidebar menu. What do you think?

Hi @alishapapun! The sidebar looks way cleaner and less distracting. It's also cool to have the search and archive pages combined! :raised_hands: Nice work on figuring this one out since it was trickier than I originally expected.

I came across one problem while testing. I wasn't able to return any messages from the search bar. I tried using names, email addresses, message identifiers, and text from public messages, but it always returned empty. Were you able to get messages to return when you tried a search?

Otherwise, I left minor feedback as in-line comments above. I also suggest pulling changes from master branch and rebasing your branch on top of changes since some other PRs were merged in the meanwhile.

Do you think you can make these changes by Wednesday? Let me know once you have a chance to look through the feedback. :) Thanks for sticking through with this one.

Metadata Update from @jflory7:
- Pull-request untagged with: needs testing
- Pull-request tagged with: needs changes

5 years ago

Metadata Update from @jflory7:
- Request assigned

5 years ago

Could id="search_fasid" be removed since it was removed from custom.css?

No, Justin. This ID is used by fas_details.js to fetch the value entered by the user to search the username, it can't be removed.

Maybe we could put this above the _message_list so it is easier to see. What do you think?

Sure! I will make the change.

Could you please choose a more a descriptive name for this CSS ID? When I read through the CSS file, it's not obvious #id_q is for the archive search bar. It should be clear what the style elements are for from the name.

The search box present in the search.html is an element of haystack.forms and the id generated by is automatically generated. So the ID of the search element in archive.html has to be the same as that of search.html since it requires the same value. Am I making sense?

Since this is the landing page and the first page the user will visit when opening the site, it might make more sense to move it to the top of the sidebar menu. What do you think?

Sure! I will rename it as Home then.

2 new commits added

  • Merge archive and search page
  • Improve side navigation design
5 years ago

rebased onto fe3f5da

5 years ago

Hi @alishapapun! The sidebar looks way cleaner and less distracting. It's also cool to have the search and archive pages combined! 🙌 Nice work on figuring this one out since it was trickier than I originally expected.
I came across one problem while testing. I wasn't able to return any messages from the search bar. I tried using names, email addresses, message identifiers, and text from public messages, but it always returned empty. Were you able to get messages to return when you tried a search?
Otherwise, I left minor feedback as in-line comments above. I also suggest pulling changes from master branch and rebasing your branch on top of changes since some other PRs were merged in the meanwhile.
Do you think you can make these changes by Wednesday? Let me know once you have a chance to look through the feedback. :) Thanks for sticking through with this one.

Hey, @jflory7 I made the required changes and tested. It's working perfectly. I also rebased onto the top of changes. When you get some time, do check out this PR. Thanks!

Metadata Update from @jflory7:
- Pull-request untagged with: needs changes
- Request assigned

5 years ago

The search box present in the search.html is an element of haystack.forms and the id generated by is automatically generated. So the ID of the search element in archive.html has to be the same as that of search.html since it requires the same value. Am I making sense?

I didn't know it was automatically generated. Thanks for pointing it out.

Hey, @jflory7 I made the required changes and tested. It's working perfectly. I also rebased onto the top of changes. When you get some time, do check out this PR. Thanks!

Awesome, everything works as expected on my end! Nice work @alishapapun. :tada: I'm going to merge this one now! :ocean:

Pull-Request has been merged by jflory7

5 years ago