Skip to content

Commit

Permalink
fix: make #heading more resilient in terms of a re-render, using prop…
Browse files Browse the repository at this point in the history 
…s as the rerender reference
  • Loading branch information
@tujoworker
tujoworker committed Jun 23, 2020
1 parent 0889241 commit 5ea59e6
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 38 deletions.
12 changes: 6 additions & 6 deletions packages/dnb-ui-lib/src/components/heading/Heading.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,6 @@ export default class Heading extends React.PureComponent {
constructor(props, context) {
super(props)

// this._id = props.id || makeUniqueId()

this._ref = React.createRef()

const state = {
Expand All @@ -177,7 +175,8 @@ export default class Heading extends React.PureComponent {
state.counter.isHeading = true
}

const { level: newLevel } = correctHeadingLevel({
state.counter = correctHeadingLevel({
ref: props, // Do that only to make shure we run the correction only if props has changed
counter: state.counter,
level: parseFloat(props.level),
inherit: isTrue(props.inherit),
Expand All @@ -193,7 +192,7 @@ export default class Heading extends React.PureComponent {

globalSyncCounter.current = state.counter

state.level = newLevel
state.level = state.counter.level
state.prevLevel = props.level

this.state = state
Expand Down Expand Up @@ -233,6 +232,7 @@ export default class Heading extends React.PureComponent {

let { size, element } = this.props
const { level } = this.state

const debug = _debug || this.context.heading?.debug
const debug_counter =
_debug_counter || this.context.heading?.debug_counter
Expand Down Expand Up @@ -295,9 +295,9 @@ Heading.Increase = (props) => <HeadingProvider increase {...props} />
Heading.Decrease = (props) => <HeadingProvider decrease {...props} />
Heading.Up = (props) => <HeadingProvider increase {...props} />
Heading.Down = (props) => <HeadingProvider decrease {...props} />
Heading.Reset = () => {
Heading.Reset = (props) => {
globalHeadingCounter.current?.reset()
return <></>
return <HeadingProvider {...props} />
}
Heading.resetLevels = resetLevels
Heading.setNextLevel = setNextLevel
Expand Down
21 changes: 21 additions & 0 deletions packages/dnb-ui-lib/src/components/heading/HeadingHelpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@ import {
export const globalSyncCounter = React.createRef()
export const globalHeadingCounter = React.createRef(null)

const refs = React.createRef()

export const correctHeadingLevel = ({
counter,
level,
ref = null,
reset = null,
inherit = null,
increase = false,
Expand All @@ -24,6 +27,18 @@ export const correctHeadingLevel = ({
bypassChecks = false,
debug = null
}) => {
// Do that only to make shure we run the correction only if props has changed
if (ref && refs.current) {
const foundRef = refs.current.find((cur) => cur.ref === ref)
if (foundRef) {
// double check, if level is provided
// if (ref.level && ref.level !== foundRef.ref.level) {
// } else {
// }
return foundRef.counter
}
}

if (bypassChecks) {
counter.enableBypassChecks()
}
Expand Down Expand Up @@ -111,6 +126,12 @@ export const correctHeadingLevel = ({
counter.disableBypassChecks()
}

// Do that only to make shure we run the correction only if props has changed
if (ref) {
refs.current = refs.current || []
refs.current.push({ ref, counter })
}

return counter
}

Expand Down
11 changes: 4 additions & 7 deletions packages/dnb-ui-lib/src/components/heading/HeadingProvider.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,6 @@ export default class HeadingProvider extends React.PureComponent {
constructor(props, context) {
super(props)

// this._id = props.id || makeUniqueId()

const state = {
context,
_listenForPropChanges: true
Expand All @@ -128,9 +126,8 @@ export default class HeadingProvider extends React.PureComponent {
state.counter.setContextCounter(globalHeadingCounter.current)
}

// state.counter.rerender = this.rerender

const { level: newLevel } = correctHeadingLevel({
state.counter = correctHeadingLevel({
ref: props,
counter: state.counter,
level: parseFloat(props.level),
inherit: isTrue(props.inherit),
Expand All @@ -147,8 +144,8 @@ export default class HeadingProvider extends React.PureComponent {
// Set the current level here, and keep it, so a heading, comming later in, will inherit it
// This will require a new Counter "group" - not the global.
// We basically start again counting from this one.
state.level = newLevel
state.prevLevel = state.newProps.level || newLevel
state.level = state.counter.level
state.prevLevel = state.newProps.level || state.counter.level
this.state = state
}

Expand Down
62 changes: 37 additions & 25 deletions packages/dnb-ui-lib/src/components/heading/__tests__/Heading.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,21 @@ class StateChanges extends React.PureComponent {
<Heading.Level group="B">
<Heading>h3 before</Heading>
{this.state.showHeading3 && (
<>
<React.StrictMode>
<Heading increase>h4 1</Heading>
<Heading>h4 2</Heading>
<Heading increase>h5 1</Heading>
</>
</React.StrictMode>
)}
<Heading>h3 after</Heading>

<Heading.Increase group="C">
{this.state.showHeading4 && (
<>
<React.StrictMode>
<Heading>h4 1</Heading>
<Heading>h4 2</Heading>
<Heading increase>h5 1</Heading>
</>
</React.StrictMode>
)}
</Heading.Increase>
</Heading.Level>
Expand All @@ -74,7 +74,7 @@ class StateChanges extends React.PureComponent {
describe('Heading component', () => {
it('have to match level correction', () => {
const Comp = mount(
<>
<React.StrictMode>
<Heading level={2} debug={warn}>
Heading #1
</Heading>
Expand All @@ -101,7 +101,7 @@ describe('Heading component', () => {
<Heading debug inherit decrease>
Heading #12
</Heading>
</>
</React.StrictMode>
)

let i = -1
Expand All @@ -122,7 +122,7 @@ describe('Heading component', () => {

it('have to match global reset', () => {
const Comp = mount(
<>
<React.StrictMode>
<Heading.Level debug={warn} reset={1}>
<Heading>Heading #1</Heading>
</Heading.Level>
Expand All @@ -135,7 +135,7 @@ describe('Heading component', () => {
<Heading debug reset>
Heading #4
</Heading>
</>
</React.StrictMode>
)

let i = -1
Expand All @@ -148,7 +148,7 @@ describe('Heading component', () => {

it('have to match context reset', () => {
const Comp = mount(
<>
<React.StrictMode>
<Heading.Level debug={warn} reset={1}>
<Heading>Heading #1</Heading>
<Heading>Heading #2</Heading>
Expand All @@ -158,7 +158,7 @@ describe('Heading component', () => {
</Heading.Level>
<Heading reset>Heading #5</Heading>
</Heading.Level>
</>
</React.StrictMode>
)

let i = -1
Expand All @@ -172,14 +172,14 @@ describe('Heading component', () => {

it('have to match level correction with manual heading', () => {
const Comp = mount(
<>
<React.StrictMode>
<Heading.Level debug={warn} reset={1}>
<Heading>Heading #1</Heading>
<Heading>Heading #2</Heading>
<H3 level="use">Heading #3</H3>
<Heading>Heading #4</Heading>
</Heading.Level>
</>
</React.StrictMode>
)

const first = Comp.find('h3.dnb-h--medium')
Expand All @@ -198,7 +198,15 @@ describe('Heading component', () => {
// resetLevels(1,{overwriteContext:true})
// resetAllLevels()
Heading.resetLevels(1, { overwriteContext: true })
const Comp = mount(<Heading debug={warn}>Heading #1</Heading>)

const RenderComp = (props) => (
<React.StrictMode>
<Heading debug={warn} {...props}>
Heading #1
</Heading>
</React.StrictMode>
)
const Comp = mount(<RenderComp />)

expect(Comp.find('.dnb-heading').at(0).text()).toBe('[h1] Heading #1')

Expand All @@ -207,7 +215,7 @@ describe('Heading component', () => {
// We got a level correction here!
expect(Comp.find('.dnb-heading').at(0).text()).toBe('[h2] Heading #1')

expect(warn).toBeCalledTimes(1)
expect(warn).toBeCalledTimes(2) // 2 because of StrictMode
expect(warn).toHaveBeenCalledWith(
'Heading levels can only be changed by factor one! Got:',
3,
Expand All @@ -224,7 +232,8 @@ describe('Heading component', () => {
Comp.setProps({ level: 4 })

expect(Comp.find('.dnb-heading').at(0).text()).toBe('[h4] Heading #1')
expect(warn).toBeCalledTimes(1) // still one time, same as we had earlier
// still one time, same as we had earlier
expect(warn).toBeCalledTimes(2) // 2 because of StrictMode
})

it('have to have correct leveling after using setNextLevel', () => {
Expand All @@ -237,11 +246,14 @@ describe('Heading component', () => {
const Comp2 = mount(<Heading debug={warn}>h2</Heading>)

setNextLevel(3, { overwriteContext: true })
const Comp3 = mount(
<Heading.Level debug={warn}>
<Heading>h3</Heading>
</Heading.Level>
const RenderComp3 = (props) => (
<React.StrictMode>
<Heading.Level debug={warn} {...props}>
<Heading>h3</Heading>
</Heading.Level>
</React.StrictMode>
)
const Comp3 = mount(<RenderComp3 />)

expect(Comp1.find('.dnb-heading').at(0).text()).toBe('[h1] h1')
expect(Comp2.find('.dnb-heading').at(0).text()).toBe('[h2] h2')
Expand Down Expand Up @@ -306,12 +318,12 @@ describe('Heading component', () => {

it('should set level if skip_correction is true', () => {
const Comp = mount(
<>
<React.StrictMode>
<Heading.Level debug={warn} skip_correction reset={1}>
<Heading level={4}>Heading #1</Heading>
<Heading increase>Heading #2</Heading>
</Heading.Level>
</>
</React.StrictMode>
)

const elem = Comp.find('.dnb-heading')
Expand All @@ -322,15 +334,15 @@ describe('Heading component', () => {
it('should not increase level above 6', () => {
resetLevels(1, { overwriteContext: true })
const Comp = mount(
<>
<React.StrictMode>
<Heading.Level debug={warn}>
<Heading>Heading #1</Heading>
<Heading.Increase skip_correction level="6">
<Heading>Heading #2</Heading>
<Heading increase>Heading #3</Heading>
</Heading.Increase>
</Heading.Level>
</>
</React.StrictMode>
)

const elem = Comp.find('.dnb-heading')
Expand Down Expand Up @@ -451,7 +463,7 @@ function makeComp() {
gComp =
gComp ||
mount(
<>
<React.StrictMode>
<Heading.Level debug={warn} reset={1}>
<Heading>Heading #1</Heading>
<Heading>Heading #2</Heading>
Expand All @@ -471,7 +483,7 @@ function makeComp() {
</Heading.Increase>
</Heading.Increase>
</Heading.Level>
</>
</React.StrictMode>
)

return gComp
Expand Down

0 comments on commit 5ea59e6

Please sign in to comment.