stevehanson / domain-switcher-chrome

Domain Switcher extension for Google Chrome
https://chrome.google.com/webstore/detail/domain-switcher/lbehdhpgigdlinfkidifkbhjnaglfojc
Other
21 stars 15 forks source link

#11 Feature templates #15

Open amenk opened 8 years ago

amenk commented 8 years ago

One Function missing: I want a popup menu entry which I can click, it will detect the current domain and use it as liveDomain and add the environment. How can I do that?

amenk commented 8 years ago

Got it :) Please check and hopefully merge ...

stevehanson commented 8 years ago

I'm a little busy the next couple days but will try to get a look at this soon. I made some notes on issue #11. Overall, I'm feeling really hesitant to complicate the UI.

amenk commented 8 years ago

Improved the UI. Can it maybe be merged now? Would be great!

stevehanson commented 8 years ago

Overall, it looks pretty good.

There are a few changes that I'd like to make before pushing it to the store:

If you're not interested in making those changes, I can go ahead and merge this and get to them. I'd like to get these changes implemented before pushing to the webstore.

stevehanson commented 8 years ago

Would also be nice to have a button on the options page "Add project using template" that launches the modal.

amenk commented 8 years ago

Thanks for the review! I think I will try to make the changes - now you make me busy ;)

This is more of a code thing: could we store the template information in $scope.templates (a new array) instead of in $scope.projects. Feel like that might keep the code a little cleaner. I like using an array, because we could in the future support picking which template to use in the quick add modal, and this would keep us from having to migrate users' localstorage data.

I was having backwards compatibility in mind (not migrating the localstorage). I just can add another array and push it to the local storage as well you mean?

stevehanson commented 8 years ago

Haha. Yup, your solution was backwards compatible—that's not what I meant. But yeah, creating another array and pushing to local storage is what I was suggesting.

Lemme know if you want to bounce any questions off me. I'm going to close issue #11. Let's use this PR for discussion instead.

amenk commented 8 years ago
  1. Live domain could be reworded "first entry" (but I think live domain makes also much sense - cause that is usually the plain one, others have added prefixes or suffixes - or do you have another example)
  2. But: I will add the quick add also for for example staging domains - then just remove staging and interpolate the live domain (I hope that works)
  3. Maybe we do not need the popup at all - I was thinking about live-completing from the template, when the first domain is entered, what do you think?
amenk commented 8 years ago

Can't come up with a nice UI for alternative 3. (I was thinking about a lock/unlock logic like in graphic software where changing the width affects the height) - will go for a popup as suggested.

amenk commented 8 years ago

Haha. Yup, your solution was backwards compatible—that's not what I meant. But yeah, creating another array and pushing to local storage is what I was suggesting.

Currently you store anything in localStorage['domanSwitcher] so should I add something like localStorage['domainSwitcherTemplates']?

amenk commented 8 years ago

Support for arbitrary string in template as opposed to full domains. I think this works, just fails if $$ is used. Not a big deal. Might just need to change terminology, instead of calling this the "live domain", we could call it the template variable or something.

hm .. okay ... the point is that I use the current domain on quick-add ... so it is meant for the live domain - sure you can use any string, but then it is a (only) little bit less awesome :)

amenk commented 8 years ago

Templates are implemented. storage is not changed as I did not get the point :-) (see above). Can you have a look again and maybe merge?

amenk commented 8 years ago

friendly-bump Nope, I do not have a calendar reminder for this, even it looks like this :-)