#152 Fixes #146 Active class is set to an element in sidebar for all pages.
Closed 5 years ago by chetanshinde911. Opened 5 years ago by chetanshinde911.
Unknown source fixes146  into  master

file modified
+2
@@ -252,3 +252,5 @@

  .socio-buttons a {

      padding: 0.9rem;

  }

+ 

+ 

file modified
+5 -6
@@ -28,16 +28,15 @@

              <ul class="nav nav-stacked">

                  <li><a>Packets sent: <b>{{ packets_sent }}</b></a></li>

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

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

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

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

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

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

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

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

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

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

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

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

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

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

-             </ul>

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

              {% if user.is_authenticated %}

                  <form id="logout" action="{% url 'oidc_logout' %}" method="post">

                      {% csrf_token %}

  1. What is the summary of your change?
    To carry out the changes suggested I carried out changes in two files.
    a) assets/css/custom.css :- this file holds css changes. for turning the background white of a selected hyperlink and
    turning the text in the hyperlink to blue and bold.
    b) template/base.html :- this file hold the logic to change the class and id of various list element present on the page
    in order to carry out the desired effect. the hyperlink which is under selection its class is been changed and id is
    been assigned to the hyperlink of that class in order to change the background color and text-weight and color.
  2. Why this change is helpful?
    This change will help the user to understand which hyperlink from the sidebar is under selection.

  3. What do you think is the outcome of this change?
    Only the tab under selection is been highlighted and other tab are not.

Open for review.

@chetanshinde911 kindly remove the css changes you have added to the file.

The redesign is currently being worked on by me, and I already have an approved design for the same.

Also, could you please rebase and squash your commits into one commit?
This helps keep the git changelog tidy. If you have never rebased before, see these two articles for help:
Don't Be Scared of git rebase
How to Rebase a Pull Request

Cool @phoenixabhishek please can you share which css changes you have carried out so that i can make changes to the css file accordingly. :)

We just need one class of 'active' to be added to achieve the desired result, the class 'inactive' can be removed and the condition can be modified accordingly.
Also, we don't need id attribute on the elements, those can be removed too.
(Please make this changes to other li elements too)

Just to let you know, using the same id on multiple elements defeats the very purpose of the id attribute.
id attributes are used to identify an element uniquely in the document.

Cool @phoenixabhishek please can you share which css changes you have carried out so that i can make changes to the css file accordingly. :)

I'll have a separate PR for those changes.
In this PR you can just push the changes on the base.html file with the comments that I have made on the file.

Please look at the comments and make the changes accordingly :smile:

So, you are writing the active class which along with highlighting will change the background of the hyperlink selected.

What my pull request should have is a basic equalif logic for setting the class of the current hyperlink under selection equal to active right? And should not assign id to the href ? right?

What my pull request should have is a basic equalif logic for setting the class of the current hyperlink under selection equal to active right? And should not assign id to the href ? right?

@chetanshinde911 yes, right.

Metadata Update from @jflory7:
- Pull-request tagged with: needs changes, new change, type - frontend, type - summer coding
- Request assigned

5 years ago

Hi @chetanshinde911, thank you for opening the pull request! :tada:

It looks like the previous discussion in the comments was helpful for clarification. Since this PR was opened, another PR was merged. This PR now has a merge conflict. To resolve this, you need to rebase the changes on master branch into the fixes146 branch. I normally do this with the following:

git checkout master
git pull upstream master
git checkout fixes146
git rebase master

This assumes you have two remotes (git remote -v), one called origin for your fork and one called upstream for the fedora-commops/fedora-happiness-packets repo. After you run the above rebase command, you should be prompted for which files have conflicts.

Of course there are other ways to resolve a merge conflict, so if this method feels confusing, I suggest searching around for more guidance. Let me know if you need further guidance.

rebased onto dc3400e

5 years ago

rebased onto f2ec50e

5 years ago

I will create a fresh pull request and close this one. @jflory7. is it ok?

@chetanshinde911 Sure, if it's easier to start fresh, it's okay to do that. :thumbsup: Let me know if I can help with anything. Some of the more complicated git tasks can be tricky to learn.

Pull-Request has been closed by chetanshinde911

5 years ago