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

Place holder render #469

Merged
merged 2 commits into from
Feb 19, 2020
Merged

Place holder render #469

merged 2 commits into from
Feb 19, 2020

Conversation

jason-appliedis
Copy link
Contributor

Bug fix? | [x ]
PlaceHolder component is not rendering after a string change in it's properties.

What's in this Pull Request?

Update to the "shouldComponentUpdate' function in placeholder allowing it to check for more property changes.

currently returns FALSE unless a user changes the width of the component OR the hide button prop is added;

Added a comparison to the props for conditional rendering of strings.

Use case:
I wanted the button label to change based on the property pane being open or not.
Ex.
<Placeholder
iconName='Edit'
iconText='Configure your web part'
description='Please configure the web part.'
buttonLabel={(this.props.isPaneOpen)?'Close Property Pane':'Configure'}
onConfigure={this.props._onConfigure}
/>
buttonLabel={(this.props.isPaneOpen)?'Close Property Pane':'Configure'} did not trigger a render().

added:
for (const property in nextProps) {
if (property != '_onConfigure'){
if (nextProps[property] != this.props[property]) {
return true;
}
}
}

To allow the user to render conditional strings in their logic.

Added a feature that allows users to choose a field width as a prop on the ListView  react control
@codecov-io
Copy link

Codecov Report

Merging #469 into dev will decrease coverage by 0.33%.
The diff coverage is 28.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #469      +/-   ##
==========================================
- Coverage   73.31%   72.98%   -0.34%     
==========================================
  Files          18       18              
  Lines         802      807       +5     
  Branches      164      157       -7     
==========================================
+ Hits          588      589       +1     
- Misses        164      166       +2     
- Partials       50       52       +2
Impacted Files Coverage Δ
src/controls/listView/IListView.ts 100% <ø> (ø) ⬆️
src/controls/placeholder/PlaceholderComponent.tsx 88.23% <25%> (-8.44%) ⬇️
src/controls/listView/ListView.tsx 55.98% <33.33%> (-0.27%) ⬇️

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 5cf8856...4035e90. Read the comment docs.

AJIXuMuK added a commit that referenced this pull request Feb 19, 2020
@AJIXuMuK AJIXuMuK merged commit be3422f into pnp:dev Feb 19, 2020
@AJIXuMuK
Copy link
Collaborator

Thank you @jason-appliedis for the change!
I've merged it manually to exclude ListView-related changes.
The changes will be available in the next drop or you can test them right away in the beta version.

@AJIXuMuK AJIXuMuK added this to the 1.17.0 milestone Feb 19, 2020
@estruyf estruyf mentioned this pull request Mar 23, 2020
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.

None yet

3 participants