Skip to content
This repository was archived by the owner on Oct 2, 2019. It is now read-only.

Disable within disabled fieldset - #772

Open
AdamAxtmann wants to merge 5 commits into
angular-ui:masterfrom
AdamAxtmann:master
Open

Disable within disabled fieldset#772
AdamAxtmann wants to merge 5 commits into
angular-ui:masterfrom
AdamAxtmann:master

Conversation

@AdamAxtmann

Copy link
Copy Markdown

disable ui-selects when inside a disabled fieldset

@AdamAxtmann AdamAxtmann changed the title Disable within fieldset Disable within disabled fieldset Mar 19, 2015
Comment thread src/uiSelectDirective.js Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

.is() isn't part of angular.element so we'll be creating a dependency on jquery to make this work which we should avoid

@rwlogel

rwlogel commented Mar 20, 2015

Copy link
Copy Markdown
Contributor

What if we did this instead:

if(!!disabledWatch.is) {
  return disabledWatch.is(':disabled');  // Use jquery if available
}
return !!element[0].querySelector('.ui-select-disabled-watch:disabled');

I tested in in Chrome and Firefox and it worked fine. It doesn't seem to work in IE 11, but neither do fieldsets.

Also maybe the unit tests shouldn't use jquery if dependency on jquery should be avoided.

@AdamAxtmann

Copy link
Copy Markdown
Author

@rwlogel That seems to work. I ran the tests on my machine and updated the pull request with the change.

@AdamAxtmann

Copy link
Copy Markdown
Author

hello?

@rwlogel

rwlogel commented Sep 4, 2015

Copy link
Copy Markdown
Contributor

Any ETA for this merge?

@Sheeryada

Copy link
Copy Markdown

can anybody update on this merge ?

@wesleycho

Copy link
Copy Markdown
Contributor

This needs history cleaned up - there should be no revert commit.

@cwagner22

Copy link
Copy Markdown

+1

@katemihalikova

Copy link
Copy Markdown
Contributor

What about merging this via squash merge?

@luzrafaelf

Copy link
Copy Markdown

+1

@Jefiozie

Copy link
Copy Markdown
Contributor

Hi, I think people see what needs to be done to get this merged. Somebody maybe can make a new PR for this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants