Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Tab demo clarity, tabIndex bugfix #1878

Merged
merged 4 commits into from
Aug 8, 2018

Conversation

Blackbaud-AlexKingman
Copy link
Contributor

Relates to #1705

Based on consumer feedback, we have determined the tabs demo could be expanded a bit more to provide better examples of how to utilize the component. @blackbaud-johnly is working on the documentation for this in a separate issue #1868 .

This changes the following:

  1. Adds a few more examples to the tab demo to better show how to effectively use tabIndex property and generate tabs bound to arrays.
  2. Fixes a bug in tabset.service.ts where any tabIndex deliberately set to 0 was being overwritten.

@Blackbaud-TrevorBurch Blackbaud-TrevorBurch self-assigned this Aug 6, 2018
@codecov-io
Copy link

codecov-io commented Aug 6, 2018

Codecov Report

Merging #1878 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1878   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         414     414           
  Lines        8640    8642    +2     
  Branches     1278    1279    +1     
======================================
+ Hits         8640    8642    +2
Impacted Files Coverage Δ
src/modules/tabs/tabset.service.ts 100% <100%> (ø) ⬆️
src/modules/tabs/tab.component.ts 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 003059c...7b1078f. Read the comment docs.

Copy link
Member

@Blackbaud-TrevorBurch Blackbaud-TrevorBurch left a comment

Choose a reason for hiding this comment

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

LGTM

@Blackbaud-TrevorBurch
Copy link
Member

Blackbaud-TrevorBurch commented Aug 6, 2018

Also addresses #1631

Copy link
Contributor

@blackbaud-conorwright blackbaud-conorwright left a comment

Choose a reason for hiding this comment

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

just one tiny style comment

(newTab)="addTabClick()"
(openTab)="openTabClick()"
[active]="activeTabIndex2"
(activeChange)="tabChanged($event)">
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you reorder these so that [active] is first?

[active]
(activeChange)
(newTab)
(openTab)

2nd comment: #1647

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants