Skip to content

RFC: toggle-visibility - #26

Closed
jamesarosen wants to merge 1 commit into
emberjs:masterfrom
jamesarosen:toggle-visibility
Closed

jamesarosen wants to merge 1 commit into
emberjs:masterfrom
jamesarosen:toggle-visibility

Conversation

@jamesarosen

Copy link
Copy Markdown

@jamesarosen jamesarosen changed the title Add RFC: toggle-visibility RFC: toggle-visibility Dec 24, 2014
@mmun

mmun commented Dec 26, 2014

Copy link
Copy Markdown
Member

View is already has a ton of methods so this seems fine. We should aim to pull visibility out into a mixin a la @ebryn's recent spikes.

@jamesarosen

Copy link
Copy Markdown
Author

Related Twitter thread: https://twitter.com/paul_irish/status/564577651609059329

Could be obviated by jquery/jquery#2057

@jamesarosen

Copy link
Copy Markdown
Author

emberjs/ember.js#9634 has some more information, including some points on accessibility.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I would prefer alternative 2, but I have two reservations:

  1. is it OK for Ember to inject <style>[hidden] { display: none; }</style> into the page on boot? Is that likely to break apps? Could it at least inject it before any other <link> or <style> tags so it's easy to override with simple CSS?
  2. are there apps or addons that rely on el.prop('style').display being "none" for hidden views? (As opposed to them just not being visible.)

@tomdale

tomdale commented Jun 7, 2015

Copy link
Copy Markdown
Member

We are trying to pare down the number of built-in APIs that abstract DOM features. In favor of this, we'd recommend people use an {{#if}} or bind this attribute using the new syntax described in #60.

@tomdale tomdale closed this Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants