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

Pf4 dark mode #1525

Merged
merged 8 commits into from Jan 16, 2020
Merged

Pf4 dark mode #1525

merged 8 commits into from Jan 16, 2020

Conversation

damianpm
Copy link
Contributor

@damianpm damianpm commented Jan 13, 2020

What this PR does / why we need it:

PatternFly is switching by default to dark theme.
See:
https://www.patternfly.org/v4/documentation/react/components/page#examples
And:
https://www.patternfly.org/v4/documentation/react/components/nav

Verification steps

npm run update-dependencies

Special notes for your reviewer:

Before:
before

After:
after

Deployed to preview env

@damianpm damianpm marked this pull request as ready for review January 13, 2020 16:10
Copy link
Contributor

@josemigallas josemigallas left a comment

Choose a reason for hiding this comment

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

The animation in vertical nav is not working. It's easier to spot when closing a menu:
dark

This is how it should look like:
light

Something wrong with pf-m-dark maybe? I couldn't find what's causing it.

@codecov
Copy link

codecov bot commented Jan 14, 2020

Codecov Report

Merging #1525 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1525      +/-   ##
=========================================
- Coverage   93.01%     93%   -0.01%     
=========================================
  Files        2516    2516              
  Lines       83600   83570      -30     
=========================================
- Hits        77757   77722      -35     
- Misses       5843    5848       +5
Impacted Files Coverage Δ
lib/tasks/multitenant/tenants.rake 41.5% <0%> (-5.67%) ⬇️
app/models/user.rb 94.42% <0%> (-0.75%) ⬇️
spec/acceptance/api/user_spec.rb
spec/acceptance/api/cms_template_spec.rb 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 b27ede2...81641e0. Read the comment docs.

@damianpm
Copy link
Contributor Author

@josemigallas I think there are no transitions or animations anymore, if you look at the examples, they don't have it:
https://www.patternfly.org/v4/documentation/react/components/nav#expandable

@thomasmaas
Copy link
Member

But there is still some sort of animation going on in preview…

@damianpm damianpm force-pushed the pf4-dark-mode branch 2 times, most recently from f5aa4c9 to 8c2dc63 Compare January 15, 2020 11:34
@damianpm damianpm force-pushed the pf4-dark-mode branch 2 times, most recently from 66332ca to 81641e0 Compare January 15, 2020 14:29
@damianpm
Copy link
Contributor Author

Merging and creating a new issue to investigate if the "jump" is a real issue or just something new in PF
https://issues.redhat.com/browse/THREESCALE-4252

@damianpm damianpm dismissed josemigallas’s stale review January 16, 2020 10:42

Creating new issue to further investigate

@damianpm damianpm merged commit c994272 into master Jan 16, 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
4 participants