Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

Hide textbox for unlisted organisation in edit profile form if an organisation is selected from given list #623

Conversation

anjali-dhanuka
Copy link
Contributor

@anjali-dhanuka anjali-dhanuka commented Feb 15, 2018

Description

If the user has selected from the listed organisations , then the unlisted organisation textbox doesn't appear.

Fixes #622

Type of Change:

  • Code

Code/Quality Assurance Only

  • New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

screenshot from 2018-02-14 10-50-33

Checklist

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Any dependent changes have been merged

Code/Quality Assurance Only

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been published in downstream modules

@anjali-dhanuka
Copy link
Contributor Author

@tonythomas01 @tapasweni-pathak Please review.

@coveralls
Copy link

coveralls commented Feb 15, 2018

Coverage Status

Coverage remained the same at 43.304% when pulling 1938553 on anjali-dhanuka:edit_profile_organisations_fix into 587ba0a on systers:develop.

</div>
</div>
<script type="text/javascript" src="{% static 'vms/js/hide_organization.js' %}"></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a new file you are introducing ? Please commit the one to app/static/ or media/ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No @tonythomas01 Its already there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool then.

<div class="col-md-8">
<input class="form-control" type="text" placeholder="{% blocktrans %}Organization{% endblocktrans %}" name="unlisted_organization" value="{% if form.unlisted_organization.value %}{{ form.unlisted_organization.value }}{% endif %}">
<div id="div_id_unlisted_organization" class="col-md-8">
<input id="id_unlisted_organization" class="form-control" type="text" placeholder="{% blocktrans %}Organization{% endblocktrans %}" name="unlisted_organization" value="{% if form.unlisted_organization.value %}{{ form.unlisted_organization.value }}{% endif %}">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ideally, id should be unique - and if there is a need for you to do this, there might be something wrong with the original design - and we might want to change it ?

https://www.w3.org/TR/html4/struct/global.html 7.5.2

div_id_unlisted_organization

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tonythomas01 input id is not needed. Thus removed them. Please review.

<label class="col-md-2 control-label">{% trans "If your organization is not listed, please provide it here:" %}</label>
<div class="col-md-8">
<div id="div_id_unlisted_organization" class="col-md-8">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oopsie. same id used twice ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tonythomas01 If I change the id then I will have to rewrite the hide_organization function again for that which wil be redundant.
image
Can't we keep the same id as they are in different templates and both have exactly same function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even though, I am wondering why you have to use this id on these two fields. When you hide the parent element, the child too gets hidden right ?

Bonus, this hide and seek in js can be replaced with http://api.jquery.com/toggle/

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tonythomas01 That is not the parent infact they reflect the same div that's why they have the same id. There is an if-else statement in between.
screenshot from 2018-02-23 21-31-08

@tapaswenipathak tapaswenipathak merged commit 6495fc8 into anitab-org:develop Feb 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants