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

Disable proxy button when proxy is added #1604

Merged
merged 15 commits into from
Sep 21, 2016
Merged

Conversation

frenchbread
Copy link
Contributor

Closes #1484

@frenchbread
Copy link
Contributor Author

TODO: add help text to disabled button

@bajiat
Copy link
Contributor

bajiat commented Sep 20, 2016

@frenchbread Why not hide the button instead of disabling?

@frenchbread
Copy link
Contributor Author

@bajiat IMHO Hiding the "Add proxy" can confuse users. I have no idea where, in that case, to put a message saying e.g. "You have already added one proxy. Can't add more for now."

Disabling the button was also suggested by @Nazarah here.

What's left is attaching a tooltip with helper text to a button. Open to suggestions.

@frenchbread frenchbread changed the title Feature/add proxy btn disabled Disable proxy button when proxy is added Sep 20, 2016
});

Template.proxies.helpers({
proxies () {
const instance = Template.instance();
return instance.proxies.get();
}
},
biggerThan (a, b) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use more meaningful name variables here, didn't quite get what is a and what is b.
Also comment will help.

@@ -273,6 +273,7 @@
"proxies_pageHeader": "Proxies",
"proxies_addProxy": "Add proxy",
"proxies_noProxiesFound": "No proxies found.",
"proxies_cannotAddMoreProxies": "You already have a Proxy available to attach your API. Please select the Proxy. You can add a new Proxy only after your have deleted the existing one.",
Copy link
Contributor

Choose a reason for hiding this comment

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

This help text is unnecessary, if we simply hide the Add Proxy button.

const proxiesCount = Proxies.find().count();

// Set button disabled if at least one proxy is already added
instance.isDisabled.set(proxiesCount >= 1);
}
});
});

Template.proxies.events({
'click #add-proxy': function (event, instance) {
Modal.show('addProxy', { proxy: {}, isEdit: false });
Copy link
Contributor

@brylie brylie Sep 21, 2016

Choose a reason for hiding this comment

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

Be aware of changes to the addProxy form. Specifically, the template has been renamed proxyForm and used for both adding and editing proxies.

#1597

@brylie brylie assigned brylie and unassigned 55 Sep 21, 2016
@brylie
Copy link
Contributor

brylie commented Sep 21, 2016

This is really close. Just clean up the help text, which is not necessary. Also, check linter errors for missing imports, etc.

# Conflicts:
#	proxies/client/proxies.html
#	proxies/client/proxies.js
@frenchbread
Copy link
Contributor Author

Changes are done

@brylie brylie assigned 55 and unassigned brylie Sep 21, 2016
@55 55 added this to the Sprint 31 milestone Sep 21, 2016
@55 55 merged commit 90b0655 into develop Sep 21, 2016
@55 55 deleted the feature/add-proxy-btn-disabled branch September 21, 2016 11:54
@55 55 removed the in progress label Sep 21, 2016
@ilarimikkonen ilarimikkonen added this to In Review in apinf/platform Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants