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

feat(NovoDataTablePagination): implement bare pagination theme #1280

Closed
wants to merge 2 commits into from

Conversation

adrianossi
Copy link
Contributor

@adrianossi adrianossi commented Apr 1, 2022

Description

Update NovoDataTablePagination with a "bare" theme that handles situations where the total number of records is unknown.

Verify that...

  • Any related demos were added and npm start and npm run build still works
  • New demos work in Safari, Chrome and Firefox
  • npm run lint passes
  • npm test passes and code coverage is increased
  • npm run build still works

Bullhorn Internal Developers

  • Run Novo Automation
Screenshots

@adrianossi adrianossi changed the title WIP: feat(NovoDataTablePagination): implement bare pagination theme feat(NovoDataTablePagination): implement bare pagination theme Apr 1, 2022
@adrianossi adrianossi marked this pull request as draft April 1, 2022 20:57
@bvkimball
Copy link
Contributor

@adrianossi Is this in response to #1267?

@bvkimball
Copy link
Contributor

@adrianossi Can you fill out the description with what and why you are trying to accomplish? maybe even screenshot/mockup of the design error.

@@ -628,6 +630,66 @@ novo-data-table-pagination {
}
}
}
&.bare {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is how we want to implement this.

  1. I think we want to move the pagination component to be a individualized component. ie. it should own its on styles.
  2. we are trying to adopt a better naming convention for our classes, for example: novo-date-table-bare-pagination. but i don't think this is right anyways

@@ -64,6 +64,43 @@ const MAX_PAGES_DISPLAYED = 5;
<span>{{ labels.next }}</span>
</novo-button>
</ng-container>
<ng-container *ngIf="theme === 'bare'">
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does it need a separate template?

@bvkimball
Copy link
Contributor

bvkimball commented Apr 6, 2022

Also can't you just use the emptyMessage template ....

 <ng-template novoTemplate="emptyMessage">
    <entity-data-table-empty-message data-automation-id="category-table-add"
      icon="ice-cream"
      message="{{ 'CM.EMPTY_JOB_CODE' | translate }}"
      label="{{ 'CM.ADD_JOB_CODE' | translate }}"
      [permission]="canAdd"
      (add)="openAddPage()"></entity-data-table-empty-message>
  </ng-template>

@adrianossi adrianossi closed this Apr 8, 2022
@adrianossi adrianossi deleted the f/barePagination branch April 22, 2022 02:19
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