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

improve logic and ui for inferring bundle and type name when saving to catalog #108

Merged
merged 3 commits into from Nov 13, 2018

Conversation

Projects
None yet
3 participants
@ahgittin
Copy link
Contributor

ahgittin commented Nov 13, 2018

previously showed "untitled" if you clilcked to expand on a new blueprint, now defaults to tidied version of display name (which is required)

@tbouron
Copy link
Member

tbouron left a comment

Make more sense to infer from the name, I like this approach @ahgittin.
I just a couple of comments before merging this.

@@ -223,3 +228,7 @@ function templateCache($templateCache) {
$templateCache.put(TEMPLATE_URL, template);
$templateCache.put(TEMPLATE_MODAL_URL, modalTemplate);
}

var bundlize = (input) => input && input.split(/[^a-zA-Z0-9]+/).filter(x => x).join('-').toLowerCase();

This comment has been minimized.

Copy link
@tbouron

tbouron Nov 13, 2018

Member

Why create this var (which should be let) ? The function should live within bundlizeProvider which you can then call directly within this file with bundlizeProvider()(input);. Actually, a better way would be to rely on the $filter service like so: $filter('bundlize')(input);

This comment has been minimized.

Copy link
@ahgittin

ahgittin Nov 13, 2018

Author Contributor

i didn't like bundlizeProvider()(textToBundlize), but $filter('bundlize')(text) is not bad, changed

<span ng-if="form.bundle.$error.pattern">The bundle ID can contains only letters, numbers as well a the following characters: <code>.</code>, <code>-</code> and <code>_</code></span>
</p>
</div>

<div class="form-group" ng-class="{'has-error': form.symbolicName.$invalid}">
<label class="control-label">Blueprint symbolic name</label>
<input ng-model="config.symbolicName" ng-disabled="state.saving" class="form-control" placeholder="E.g my-catalog-id" name="symbolicName" required ng-pattern="state.pattern" autofocus />
<input ng-model="config.symbolicName" ng-disabled="state.saving" class="form-control" name="symbolicName" ng-pattern="state.pattern" autofocus placeholder="{{ config.name | bundlize }}"/>

This comment has been minimized.

Copy link
@tbouron

tbouron Nov 13, 2018

Member

I get that config.name is required but until you set it, there will be no placeholder. So you should do {{(config.name | bundlize) || 'E.g my-catalog-id'}}

This comment has been minimized.

Copy link
@ahgittin

ahgittin Nov 13, 2018

Author Contributor

good spot, fitxed

@@ -57,19 +60,17 @@ <h3 class="modal-title">{{getTitle()}}</h3>
<label class="control-label">Bundle ID</label>
<div class="input-group">
<span class="input-group-addon">catalog-bom-</span>
<input ng-model="config.bundle" ng-disabled="state.saving" class="form-control" placeholder="E.g my-bundle" name="bundle" required ng-pattern="state.pattern" autofocus />
<input ng-model="config.bundle" ng-disabled="state.saving" class="form-control" name="bundle" ng-pattern="state.pattern" autofocus placeholder="{{ config.name | bundlize }}"/>

This comment has been minimized.

Copy link
@tbouron

tbouron Nov 13, 2018

Member

I get that config.name is required but until you set it, there will be no placeholder. So you should do {{(config.name | bundlize) || 'E.g my-bundle'}}

ahgittin added some commits Nov 13, 2018

address PR comments
- reference filter function using $filter
- add placeholder content for when there is no display name set
@ahgittin

This comment has been minimized.

Copy link
Contributor Author

ahgittin commented Nov 13, 2018

good comments, all addressed

@tbouron
Copy link
Member

tbouron left a comment

LGTM, thanks @ahgittin. I'll merge once Jenkins is happy

@asfgit asfgit merged commit 6f028c8 into apache:master Nov 13, 2018

1 check passed

Jenkins: brooklyn-ui-pull-request SUCCESS 10 tests run, 0 skipped, 0 failed.
Details

asfgit pushed a commit that referenced this pull request Nov 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.