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

MBS-10357: React edit diffs #1084

Merged
merged 10 commits into from Sep 23, 2019

Conversation

@mwiencek
Copy link
Member

commented May 27, 2019

@mwiencek mwiencek force-pushed the mwiencek:react-edit-diffs branch 2 times, most recently from 6da01ec to 0d82a93 Jun 3, 2019
@mwiencek mwiencek changed the title React edit diffs MBS-10357: React edit diffs Sep 11, 2019
mwiencek added 7 commits Sep 5, 2018
 * Handle non-array children.
 * Remove the incorrect reduce, which called reactTextContent with the
   wrong argument order.
If there's no class to apply, just output the text directly.
This is only used for relationship type descriptions.

Diffing HTML is complicated, and porting display_html_diff to React for
just this single case is not worth the effort. It even seems more useful
to me to show the raw HTML, since these are technical edits by their
nature and it makes more clear exactly what was changed.
EntityLink currently tries to see if there's a name variation by
comparing the entity name with the text content being displayed. Getting
the text content of a React element tree is tricky enough (see the
`reactTextContent` implementation), but even then, it doesn't work for
custom component classes: they aren't rendered so we don't know what
their children are yet. Thus, we have to allow setting `nameVariation`
where we know one exists and where it can't be guessed otherwise.
@mwiencek mwiencek force-pushed the mwiencek:react-edit-diffs branch from 0d82a93 to 65e1aa0 Sep 18, 2019
@mwiencek

This comment has been minimized.

Copy link
Member Author

commented Sep 18, 2019

This now finishes converting the (non-historic) relationship edit diffs.

@mwiencek mwiencek force-pushed the mwiencek:react-edit-diffs branch 5 times, most recently from f4fd204 to 2a21fa3 Sep 18, 2019
This is an initial/naïve port of MusicBrainz::Server::Plugin::Diff to
JavaScript/React, minus diff_html_side, which will be removed in a
follow-up commit once the relationship edit diffs are converted.
(Similar to how the release events diffing is done here, we will not
be diffing HTML for relationships either.)

Mostly resolves MBS-10357.
@mwiencek mwiencek force-pushed the mwiencek:react-edit-diffs branch from 2a21fa3 to 8ce5a25 Sep 19, 2019
Copy link
Member

left a comment

This seems generally fine, so if it has been tested, we should merge and check further in beta :)

@yvanzo
yvanzo approved these changes Sep 23, 2019
Copy link
Contributor

left a comment

Tested as much as i can! 🚢

mwiencek added 2 commits Oct 7, 2018
Allows for interpolating long link phrases in JavaScript such that e.g.
"{entity0} is related to {entity1}" can be displayed with links to
entity0 and entity1, similar to what is done in
Entity::Relationship::verbose_phrase_with_placeholders.
Finishes MBS-10357
@mwiencek mwiencek force-pushed the mwiencek:react-edit-diffs branch from 8ce5a25 to 66f9298 Sep 23, 2019
@mwiencek

This comment has been minimized.

Copy link
Member Author

commented Sep 23, 2019

Just force-pushed some commit message/code comment changes, no code changes.

@mwiencek mwiencek merged commit 7a5f3c1 into metabrainz:master Sep 23, 2019
4 checks passed
4 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: js Your tests passed on CircleCI!
Details
ci/circleci: perl-and-pgtap Your tests passed on CircleCI!
Details
ci/circleci: selenium Your tests passed on CircleCI!
Details
@mwiencek mwiencek deleted the mwiencek:react-edit-diffs branch Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.