Skip to content

Feature/hide org icons #2079

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.

Sign up for GitHub

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

Merged
marla-singer merged 4 commits into from
Feb 8, 2017
Merged

Feature/hide org icons #2079

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d075421
Check if icons are needed
Feb 7, 2017
a375d8e
Indentation
Feb 7, 2017
5b58e1a
Check if description exist
Feb 7, 2017
35f4f1e
Fix typo
Feb 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 27 additions & 19 deletions organizations/client/profile/header/header.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ <h1 id="organization-name">
{{/ if }}
</h1>
</div>
<div class="row">
<p>
{{ organization.description }}
</p>
</div>
{{# if organization.contact.person }}
Copy link
Copy Markdown
Contributor

@marla-singer marla-singer Feb 7, 2017

Choose a reason for hiding this comment

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

@NNN Why do you use this condition for description area?

Copy link
Copy Markdown
Contributor Author

@55 55 Feb 7, 2017

Choose a reason for hiding this comment

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

Redundant space if description is not available.

Copy link
Copy Markdown
Contributor

@marla-singer marla-singer Feb 7, 2017

Choose a reason for hiding this comment

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

So what? If organization have the description but don't have the contact person, the area will be empty instead of has description text.

Copy link
Copy Markdown
Contributor

@marla-singer marla-singer Feb 7, 2017

Choose a reason for hiding this comment

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

I think it's okay with redundant space. In future, it will have the cover photo or something else. Don't delete redundant space

Copy link
Copy Markdown
Contributor Author

@55 55 Feb 7, 2017

Choose a reason for hiding this comment

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

It also hides empty .row.
From design perspective:

  • without a condition:
    localhost 3000 organizations test 1

  • with a condition:
    localhost 3000 organizations test 2

Copy link
Copy Markdown
Contributor

@marla-singer marla-singer Feb 7, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

If organization have the description but don't have the contact person, the area is empty instead of has description text.

Copy link
Copy Markdown
Contributor Author

@55 55 Feb 7, 2017

Choose a reason for hiding this comment

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

And the idea is if we hide something, we need to also to hide markup around this element.

Copy link
Copy Markdown
Contributor

@marla-singer marla-singer Feb 7, 2017

Choose a reason for hiding this comment

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

So change the condition

Copy link
Copy Markdown
Contributor Author

@55 55 Feb 7, 2017

Choose a reason for hiding this comment

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

I am so sorry, haven't notice a typo. Now fixed.

<div class="row">
<p>
{{ organization.description }}
</p>
</div>
{{/ if }}
<div class="row">
<p>
<i class="fa fa-link" aria-hidden="true"></i>
Expand All @@ -32,20 +34,26 @@ <h1 id="organization-name">
</div>
<div class="row">
<ul class="list-inline">
<li>
<i class="fa fa-user" aria-hidden="true"></i>
{{ organization.contact.person }}
</li>
<li>
<i class="fa fa-phone" aria-hidden="true"></i>
{{ organization.contact.phone }}
</li>
<li>
<i class="fa fa-envelope" aria-hidden="true"></i>
<a href="mailto:{{ organization.contact.email }}">
{{ organization.contact.email }}
</a>
</li>
{{# if organization.contact.person }}
<li>
<i class="fa fa-user" aria-hidden="true"></i>
{{ organization.contact.person }}
</li>
{{/ if }}
{{# if organization.contact.phone }}
<li>
<i class="fa fa-phone" aria-hidden="true"></i>
{{ organization.contact.phone }}
</li>
{{/ if }}
{{# if organization.contact.email }}
<li>
<i class="fa fa-envelope" aria-hidden="true"></i>
<a href="mailto:{{ organization.contact.email }}">
{{ organization.contact.email }}
</a>
</li>
{{/ if }}
</ul>
</div>
</div>
Expand Down