Skip to content
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.

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

Use the umb-toggle directive for the "True/False" property editor #2667

Merged
merged 1 commit into from
Jun 5, 2018
Merged

Conversation

BatJan
Copy link
Contributor

@BatJan BatJan commented Jun 1, 2018

Prerequisites

  • I have written a descriptive pull-request title
  • I have linked this PR to an issue on the tracker at http://issues.umbraco.org

Description

Link to issue tracker: http://issues.umbraco.org/issue/U4-11408

I have added the umb-toggle directive in the views of the property editor and the property prevalue editor and I have added a toggle() method in the controllers so update the toggle with true/false values and made sure that the value for the modal i still saved as a string.

I also removed the $watch method from the controller of the property editor saving at little bit of CPU power :-)

Let me know if there are some parts of the code that I should improve. I have tested that the states are svaed correctly both when setting up the datatype and also when using it on a page. The value converters still work too.

@poornimanayar
Copy link
Contributor

Hi,

Many thanks for the PR. We will review the PR within the next 14 days.

Regards,
Poornima

@poornimanayar
Copy link
Contributor

Hi,

I checked out the PR locally but I am unable to see this in action. I am not sure whether I am being silly here but from what I understand the True/False checkbox should be a shiny toggle switch?

Poornima

@BatJan
Copy link
Contributor Author

BatJan commented Jun 4, 2018

Hi Poornima

Could you post a screendump of what you're looking at? I'll have a look when I get home from work and then update this thread with some images so we can wipe out any misunderstandings and confusion :)

/Jan

@nul800sebastiaan
Copy link
Member

nul800sebastiaan commented Jun 4, 2018

@poornimanayar Did you remember to run the Gulp build? And maybe you need to update the version number in ClientDependency.config to clear some browser caches?

@poornimanayar
Copy link
Contributor

Good catch @nul800sebastiaan . I ll get the gulp build done.

@poornimanayar
Copy link
Contributor

@nul800sebastiaan That got it up and running. I shall try and get going on this.

Poornima

Copy link
Contributor

@poornimanayar poornimanayar left a comment

Choose a reason for hiding this comment

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

This is an improvement over what we have.

@poornimanayar
Copy link
Contributor

image

image

@nul800sebastiaan nul800sebastiaan merged commit 08dd743 into umbraco:dev-v7 Jun 5, 2018
@nul800sebastiaan
Copy link
Member

Cool stuff! Thanks @BatJan !

@BatJan
Copy link
Contributor Author

BatJan commented Jun 5, 2018

YES! Awesome - Thanks folks!

That feeling ;-)

alt text

@nul800sebastiaan
Copy link
Member

😂 😂

prinception

@poornimanayar
Copy link
Contributor

poornimanayar commented Jun 5, 2018 via email

@emmaburstow
Copy link
Contributor

And this is why I'm so happy to be part of this team! :)

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.

4 participants