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

fix(Select) : Clean up select footer, convert to header for UX/ease-o… #78

Merged
merged 1 commit into from
Jun 28, 2016

Conversation

escarre
Copy link
Contributor

@escarre escarre commented Jun 25, 2016

This PR makes changes requested in Issue #72 . It also moves the footer from the base of the select list to the top - with the possibility of a long list, a user might not scroll to the bottom or be aware of the 'Add New Item' capability.

What did you change?
  • footerConfig has been renamed to headerConfig
  • header is an li not a div
  • left aligns '+ Add New Item' with other li items
  • repositions as a header to solve other UX issues
Reviewers

@jgodi

Checklist (completed via merger)
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the contributing guide
  • Passes all existing automated tests
  • New functions include new tests
  • New functions are documented (with a description, list of inputs, and expected output)
  • Visually tested in supported browsers and devices
    …f-use

Fix issue #72.

Breaking Changes: Renames Select footerConfig to Select headerConfig

@jgodi
Copy link
Contributor

jgodi commented Jun 26, 2016

Looking into the failed tests on my end, probably didn't set something up right :(

@jgodi
Copy link
Contributor

jgodi commented Jun 27, 2016

For this to work now, you will have to add the following to Select.spec.js and Pagination.spec.js

import { NOVO_ELEMENTS_LABELS_PROVIDERS } from './../../novo-elements';
beforeEachProviders(() => [NOVO_ELEMENTS_LABELS_PROVIDERS]);

since we are injecting that LabelService now.

@@ -11,6 +11,8 @@ export class NovoLabelService {
quickNoteEmpty = 'No results to display...';
required = 'Required';
numberTooLarge = 'Number is too large';
selectHeaderSave = 'Save';
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make these more generic. aka save and cancel

@escarre escarre force-pushed the bug/select-footer branch 3 times, most recently from 2916e20 to 237dd2e Compare June 28, 2016 14:42
<input autofocus type="text" [placeholder]="headerConfig.placeholder" [attr.id]="name" autocomplete="false" [(ngModel)]="header.value" [ngClass]="{invalid: !header.valid}"/>
<footer>
<button (click)="toggleHeader($event, false)">{{labels.selectHeaderCancel}}</button>
<button (click)="saveHeader()" class="primary">{{labels.selectHeaderSave}}</button>
Copy link
Contributor

Choose a reason for hiding this comment

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

save/cancel?

…f-use

Fix issue bullhorn#72.

Breaking Changes: Renames Select footerConfig to Select headerConfig
@jgodi jgodi merged commit 1909cb3 into bullhorn:master Jun 28, 2016
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.

2 participants