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

BB-373 React fontawesome — entity image #350

Merged
merged 4 commits into from
Apr 8, 2020

Conversation

prabalsingh24
Copy link
Contributor

@prabalsingh24 prabalsingh24 commented Jan 26, 2020

Problem

https://tickets.metabrainz.org/browse/BB-373
https://tickets.metabrainz.org/browse/BB-386
Migrating React-FontAwesome : entities/image.js

Note from MonkeyDo: I also hijacked the PR to finish cleaning up the unused FontAwesome files made obsolete by the package change (BB-386)

Areas of Impact

src/client/components/pages/entities/image.js

@coveralls
Copy link

coveralls commented Jan 26, 2020

Coverage Status

Coverage decreased (-0.09%) to 44.259% when pulling f8c7d4e on prabalsingh24:react-fontawesome-BB373 into bf6c741 on bookbrainz:master.

Copy link
Contributor

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

This one's going to be slightly trickier.
You'll have to dig into layered icons: if an entity is deleted, it should show the entity icon and overlapped by a big red slash icon.
Docs are here: https://github.com/FortAwesome/react-fontawesome#advanced

Here's what I'm currently seeing:
Capture d’écran 2020-01-29 à 16 39 18
(You can compare it with https://test.bookbrainz.org/work/0527de58-7da4-4afd-8ba5-ba351aee1e28 for example)

@MonkeyDo
Copy link
Contributor

MonkeyDo commented Jan 29, 2020

If you're getting a 500 error when trying to visit an entity, that's because of a mismatch of bookbrainz-data package with the ORM update.

Easy fix: merge or rebase master and it should work.

@MonkeyDo MonkeyDo changed the title React fontawesome bb373 BB-373 React fontawesome — entity image Feb 12, 2020
@MonkeyDo
Copy link
Contributor

@prabalsingh24 I know you're already well busy writing tests and new PRs, but I'd love it if we could focus on closing up the FontAwesome job (finishing replacing the library, cleanup, and figuring out vertical alignment of the icons).
Let me know if you need a hand

@prabalsingh24
Copy link
Contributor Author

I will finish this asap. :) Sorry for the delay

@MonkeyDo
Copy link
Contributor

I will finish this asap. :) Sorry for the delay

No problem, I just wanted to make sure they're not forgotten, and to tie our loose ends :)

@MonkeyDo
Copy link
Contributor

MonkeyDo commented Apr 6, 2020

I'd love to close up the FontAwesome update chapter before publishing a new version of the website.
Do you want me to have a look at finishing up the remaining PRs?

prabalsingh24 and others added 3 commits April 6, 2020 18:50
Auto-import the FA css, and some tweaks to make the layers work and prevent FOUC
This should conclude moving to the official FontAwesome package
@MonkeyDo
Copy link
Contributor

MonkeyDo commented Apr 8, 2020

MonkeyDo approved these changes

@MonkeyDo MonkeyDo merged commit c3949ec into metabrainz:master Apr 8, 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.

3 participants