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.

Sign up for GitHub

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

Jump to bottom

Replace static CSS class with a data attribute on Preview #344

Merged
merged 2 commits into from
Mar 6, 2017
Merged

Replace static CSS class with a data attribute on Preview #344

merged 2 commits into from
Mar 6, 2017

Conversation

n1313
Copy link
Collaborator

@n1313 n1313 commented Mar 2, 2017

Fixes #334

@codecov-io
Copy link

codecov-io commented Mar 2, 2017 •
edited
Loading

Codecov Report

Merging #344 into next will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             next     #344   +/-   ##
=======================================
  Coverage   94.77%   94.77%           
=======================================
  Files          75       75           
  Lines         995      995           
  Branches      202      202           
=======================================
  Hits          943      943           
  Misses         52       52
Impacted Files Coverage Δ
...rc/rsg-components/Playground/PlaygroundRenderer.js 90% <ø> (ø) ✅

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 02aba04...6362158. Read the comment docs.

sapegin
sapegin reviewed Mar 2, 2017
View reviewed changes
src/rsg-components/Playground/PlaygroundRenderer.js Outdated
@@ -75,7 +74,7 @@ export function PlaygroundRenderer({
}) {
return (
<div className={classes.root}>
<div className={cx(classes.preview, 'rsg--example-preview')}>
<div className={classes.preview} data-preview={name}>
<div className={classes.isolatedLink}>
{name && (
Copy link
Member

Choose a reason for hiding this comment

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

Looks like name can be empty but I’m not sure why. Maybe we can remove this condition ;-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we can, but that's out of scope of this PR :)

Copy link
Member

Choose a reason for hiding this comment

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

Then you need to handle case when name is not there ;-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, now I see what you are talking about. OK!

@n1313
Copy link
Collaborator Author

n1313 commented Mar 6, 2017

If name is not defined then data-preview attribute will be present but empty.

@sapegin sapegin merged commit 556a5d8 into next Mar 6, 2017
@sapegin sapegin deleted the data-preview branch March 6, 2017 15:49
@sapegin
Copy link
Member

sapegin commented Mar 6, 2017

Thanks!

sapegin pushed a commit that referenced this pull request Mar 29, 2017
@n1313 @sapegin
Replace static CSS class with a data attribute on Preview (#344)
bd46636 
Fix #334
This was referenced Apr 7, 2022
Open
Open
@Bhanditz Bhanditz mentioned this pull request Nov 5, 2022
Open
@kopax kopax mentioned this pull request Nov 5, 2022
Open
@Bhanditz Bhanditz mentioned this pull request Dec 25, 2022
Open
@kopax kopax mentioned this pull request Dec 26, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sapegin sapegin sapegin left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@n1313 @codecov-io @sapegin