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

Convert entity sidebars to React #623

Merged
merged 63 commits into from Oct 11, 2018
Merged

Conversation

mwiencek
Copy link
Member

@mwiencek mwiencek commented Mar 1, 2018

No description provided.

@mwiencek mwiencek force-pushed the sidebar-react branch 2 times, most recently from 6707f72 to 6ff94de Compare March 26, 2018 07:23
@mwiencek mwiencek changed the title Convert entity sidebars to React [WIP] Convert entity sidebars to React Mar 26, 2018
@mwiencek
Copy link
Member Author

mwiencek commented Mar 26, 2018

No longer WIP. All of the core entitiessidebars are done now and I've split up the commits. We can do the cdstub, collection, and edit sidebars in a later PR.

root/layout/components/sidebar/ReleaseSidebar.js Outdated Show resolved Hide resolved
root/layout/components/sidebar/ReleaseSidebar.js Outdated Show resolved Hide resolved
root/layout/components/sidebar/ReleaseSidebar.js Outdated Show resolved Hide resolved
root/components/CritiqueBrainzLinks.js Show resolved Hide resolved
@mwiencek
Copy link
Member Author

mwiencek commented Oct 5, 2018

This also now converts the collection, edit, and cdstub sidebars, which were the only ones left.

This can't be imported at the top-level on the server, since jquery-ui
needs a window to exist.
$c->json has allow_blessed and convert_blessed enabled, so we can now
serialize entities that contain blessed objects like cover art.
The basic idea was that CDStubTOC modeled cdtoc_raw, and CDStub modeled
release_raw. But there's no clear reason why these are separate models,
since the tables have a 1:1 mapping (I'm not actually sure why they're
separate tables). You can't submit a CD stub without a disc ID. So these
models only serve to make the code confusing as to when you should
expect which entity.

For example, the CDStub entity class previously had discid and
track_count properties as shortcuts for the indexed search, which didn't
construct a full CDStubTOC in `schema_fixup`. These shortcuts made it
unclear when, for example, $cdstubtoc->discid was defined as opposed to
$cdstubtoc->cdstub->discid, or if $cdstubtoc->cdstub could always be
expected to be loaded.
The component uses different (and probably harder to translate) strings
than the expiration_time TT macro, and wraps the result in bracketed,
which the macro didn't do.

This commit also renames the component to ExpirationTime to match the TT
macro, which makes more sense.
This encompasses the edit page sidebar.
For hydrated components that take an entity prop, we were serializing
the entire entity (potentially including a giant list of relationships,
mediums, etc.) even though we only needed a few bits of data from it
(such as the entity type and gid).
Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

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

Looks terably good to me!

@mwiencek mwiencek merged commit 3ba1376 into metabrainz:master Oct 11, 2018
@mwiencek mwiencek deleted the sidebar-react branch October 11, 2018 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants