Add brightness and contrast slider for images #771

edmundtong · 2017-11-25T01:22:07Z

Summary:
Fixes #680.
Chose to use sliders instead of numeric minimum and maximum window
values for ease of use and implementation.

Test Plan:
Screenshot of the feature in action.

Summary: Fixes tensorflow#680. Chose to use sliders instead of numeric minimum and maximum window values for ease of use and implementation. Test Plan: Screenshot of the feature in action. ![screenshot using the image demo with decreased brightness and increased contrast](https://user-images.githubusercontent.com/14915474/33226104-974bf2c6-d153-11e7-86bf-2e3c60141e7f.png)

googlebot · 2017-11-25T01:22:10Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If your company signed a CLA, they designated a Point of Contact who decides which employees are authorized to participate. You may need to contact the Point of Contact for your company and ask to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the project maintainer to go/cla#troubleshoot.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again.

edmundtong · 2017-11-25T01:28:31Z

I signed it!

googlebot · 2017-11-25T01:28:33Z

CLAs look good, thanks!

wchargin · 2017-11-25T01:31:46Z

@chihuahua mind taking a look at this?

(I know @edmundtong and paired this with him (he's interested in contributing to TensorBoard), so I'd like someone else to review it.)

chihuahua

Sorry for the late response. I am a big fan of this feature.

Could you add a button next to each slider that

lets the user reset to default settings.
somehow informs the user if the current settings do not match the default (ie, maybe the button is only active and has an orange outline around it if the current settings do not match the default ones).

chihuahua · 2017-11-28T23:03:51Z

tensorboard/plugins/image/tf_image_dashboard/tf-image-dashboard.html

+            step="1"
+            value="{{_contrastPercentage}}"
+            immediate-value="{{_contrastPercentage}}"
+            style="width: 100%;"


Could we use embedded instead of inline styles? That decouples specifying the HTML from its design and makes it easier to make large-scale design changes.

jart · 2017-11-28T23:15:34Z

I like this change. Thank you for sending it to us.

edmundtong · 2017-11-30T00:48:20Z

Thanks! I'll get to this during the weekend.

chihuahua

Thank you! Just a couple of small nits. Also, could you upload a screenshot?

chihuahua · 2017-12-02T22:06:31Z

tensorboard/plugins/image/tf_image_dashboard/tf-image-dashboard.html

+      .resettable-slider-container paper-button {
+        flex-grow: 0;
+      }
+      paper-button[disabled] {


Could we confine this to only paper-buttons within resettable-slider-container like the other specifications?

chihuahua · 2017-12-02T22:07:51Z

tensorboard/plugins/image/tf_image_dashboard/tf-image-dashboard.html

+        this._brightnessAdjustment = 1;
+      },
+      _resetContrast() {
+        this._contrastPercentage = 100;


Could we store the 2 default values in a readOnly property of the component? ie,

_defaultBrightnessAdjustment: { type: Number, value: 1, readOnly: true, },

edmundtong · 2017-12-20T10:29:15Z

Screenshot:

chihuahua

Awesome. Could you rebase to the latest changes and then resolve the conflict?

googlebot · 2017-12-23T05:04:09Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

edmundtong · 2017-12-23T05:13:23Z

Thanks for finishing the merge, Chi! The current state is correct. Is there anything else I should do for this PR?

chihuahua · 2017-12-23T05:21:29Z

@edmundtong, I tried to resolve a merge conflict here and unfortunately triggered the googlebot. Assuming your master branch is up to date, could you try running

git rebase -i master

and then squashing the merges into the first commit? Then, force push? ie, git push --force. Sorry about that. :(

edmundtong · 2017-12-23T05:23:15Z

No problem! The edits I made were actually done on GitHub, since my computer has been acting up, and will take a little while to fix, so I'll get to this when I get my computer back up and running.

chihuahua · 2017-12-23T05:29:08Z

@jart, maybe restarting googlebot could work?

Test Plan: ![image](https://user-images.githubusercontent.com/14915474/33519823-d4f08a9a-d77c-11e7-8145-b5f710b4e976.png) Clicked buttons, saw that they're disabled appropriately and did indeed reset.

Summary: Add default values for brightness and contrast. Modify JavaScript to use those values for reset.

chihuahua · 2018-01-05T19:55:39Z

Thank you, @jart! :)

wchargin requested review from chihuahua and wchargin November 25, 2017 01:30

wchargin added the plugin:images label Nov 25, 2017

wchargin removed their request for review November 25, 2017 01:31

chihuahua reviewed Nov 28, 2017

View reviewed changes

chihuahua requested changes Dec 2, 2017

View reviewed changes

chihuahua requested changes Dec 20, 2017

View reviewed changes

Merge

0423c49

chihuahua approved these changes Dec 23, 2017

View reviewed changes

edmundtong added 3 commits December 26, 2017 23:00

Add reset button for sliders

619b999

Test Plan: ![image](https://user-images.githubusercontent.com/14915474/33519823-d4f08a9a-d77c-11e7-8145-b5f710b4e976.png) Clicked buttons, saw that they're disabled appropriately and did indeed reset.

Add imports for paper-button and paper-slider

cb81c50

Add default values

686edb8

Summary: Add default values for brightness and contrast. Modify JavaScript to use those values for reset.

edmundtong force-pushed the image-brightness-contrast branch from 98b97df to 686edb8 Compare December 26, 2017 15:02

jart merged commit d46f60f into tensorflow:master Jan 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add brightness and contrast slider for images #771

Add brightness and contrast slider for images #771

edmundtong commented Nov 25, 2017

googlebot commented Nov 25, 2017

edmundtong commented Nov 25, 2017

googlebot commented Nov 25, 2017

wchargin commented Nov 25, 2017 •

edited

Loading

chihuahua left a comment

chihuahua Nov 28, 2017

jart Nov 28, 2017

jart commented Nov 28, 2017

edmundtong commented Nov 30, 2017

chihuahua left a comment

chihuahua Dec 2, 2017

chihuahua Dec 2, 2017

edmundtong commented Dec 20, 2017

chihuahua left a comment

googlebot commented Dec 23, 2017

edmundtong commented Dec 23, 2017

chihuahua commented Dec 23, 2017 •

edited

Loading

edmundtong commented Dec 23, 2017

chihuahua commented Dec 23, 2017

chihuahua commented Jan 5, 2018

Add brightness and contrast slider for images #771

Add brightness and contrast slider for images #771

Conversation

edmundtong commented Nov 25, 2017

googlebot commented Nov 25, 2017

edmundtong commented Nov 25, 2017

googlebot commented Nov 25, 2017

wchargin commented Nov 25, 2017 • edited Loading

chihuahua left a comment

Choose a reason for hiding this comment

chihuahua Nov 28, 2017

Choose a reason for hiding this comment

jart Nov 28, 2017

Choose a reason for hiding this comment

jart commented Nov 28, 2017

edmundtong commented Nov 30, 2017

chihuahua left a comment

Choose a reason for hiding this comment

chihuahua Dec 2, 2017

Choose a reason for hiding this comment

chihuahua Dec 2, 2017

Choose a reason for hiding this comment

edmundtong commented Dec 20, 2017

chihuahua left a comment

Choose a reason for hiding this comment

googlebot commented Dec 23, 2017

edmundtong commented Dec 23, 2017

chihuahua commented Dec 23, 2017 • edited Loading

edmundtong commented Dec 23, 2017

chihuahua commented Dec 23, 2017

chihuahua commented Jan 5, 2018

wchargin commented Nov 25, 2017 •

edited

Loading

chihuahua commented Dec 23, 2017 •

edited

Loading