Skip to content

Feature/hide org icons#2079

Merged
marla-singer merged 4 commits intodevelopfrom
feature/hide-org-icons
Feb 8, 2017
Merged

Feature/hide org icons#2079
marla-singer merged 4 commits intodevelopfrom
feature/hide-org-icons

Conversation

@55
Copy link
Copy Markdown
Contributor

@55 55 commented Feb 7, 2017

Closes #2057

@55 55 added this to the Sprint 36 milestone Feb 7, 2017
{{ organization.description }}
</p>
</div>
{{# if organization.contact.person }}
Copy link
Copy Markdown
Contributor

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

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

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

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

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

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

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

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

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.

@marla-singer marla-singer self-assigned this Feb 7, 2017
@marla-singer marla-singer merged commit 38fc051 into develop Feb 8, 2017
@55 55 deleted the feature/hide-org-icons branch February 14, 2017 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants