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

refactor(bar-styling): remove glamorous div from header bars #588

Merged
merged 2 commits into from
Nov 15, 2019

Conversation

micleners
Copy link
Member

Replaced styled components with style objects. Applied styles with css prop.

re #571

Replaced styled components with style objects. Applied styles with css prop.

re dsmjs#571
@codecov
Copy link

codecov bot commented Nov 14, 2019

Codecov Report

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

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #588   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          39     39           
  Lines         191    186    -5     
=====================================
- Hits          191    186    -5
Impacted Files Coverage Δ
src/atoms/header-bars/dsmJS/component.js 100% <100%> (ø) ⬆️
src/atoms/header-bars/sponsor/component.js 100% <100%> (ø) ⬆️
src/atoms/header-bars/recurrence/component.js 100% <100%> (ø) ⬆️
src/atoms/header-bars/location/component.js 100% <100%> (ø) ⬆️
src/organisms/layout/index.js 100% <0%> (ø) ⬆️

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 517487f...085b2df. Read the comment docs.

@@ -7,6 +7,6 @@ suite('dsmJS bar', () => {
test('that dsmJS is shown', () => {
const wrapper = shallow(<DsmJsBar />);

assert.equal(wrapper.dive().text(), 'dsmJS');
assert.equal(wrapper.shallow().text(), 'dsmJS');
Copy link
Member

Choose a reason for hiding this comment

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

i think the .shallow() can just be removed here and in the other cases

Suggested change
assert.equal(wrapper.shallow().text(), 'dsmJS');
assert.equal(wrapper.text(), 'dsmJS');

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh okay! I'm not familiar with the testing platform 🙈 dive was giving errors. Thanks :)

Copy link
Member

@travi travi left a comment

Choose a reason for hiding this comment

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

the assertions should be able to be simplified by removing the second shallow call

@micleners
Copy link
Member Author

Unnecessary shallows have been removed

@travi travi merged commit c5dcd9c into dsmjs:master Nov 15, 2019
@dsmjs-bot
Copy link
Member

🎉 This PR is included in version 3.5.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@dsmjs-bot dsmjs-bot added the released This issue or pull-request was included in a version released by semantic-release label Apr 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue or pull-request was included in a version released by semantic-release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants