Skip to content

feat: Compare files side by side - #1835

Merged
juliusknorr merged 2 commits into
masterfrom
feat/version-comparison
Aug 8, 2023
Merged

juliusknorr merged 2 commits into
masterfrom
feat/version-comparison

Conversation

@juliusknorr

@juliusknorr juliusknorr commented Aug 3, 2023 •

Copy link
Copy Markdown
Member

Contributes to nextcloud/server#39140

@juliusknorr

Copy link
Copy Markdown
Member Author

@skjnldsv I'd much appreciate an early review even though still in draft.

One topic I was unsure about was how we want to determine which handlers support the comparison view. For now I keep a allow list in https://github.com/nextcloud/server/pull/39171/files#diff-8c16136a0fe1baf1357890af01439aebe1da2380f96f80afb1eab039157311b8R111 but was thinking if we should just have a supportsComparison property on the handler registration instead.

@juliusknorr juliusknorr added enhancement New feature or request 2. developing Work in progress labels Aug 3, 2023
@juliusknorr juliusknorr added this to the Nextcloud 28 milestone Aug 3, 2023
@skjnldsv

skjnldsv commented Aug 3, 2023

Copy link
Copy Markdown
Member

if we should just have a supportsComparison property on the handler registration instead.

Defnitely!
Every handler should provide if they supports comparison or not 馃憤

Comment thread src/views/Viewer.vue Outdated
@juliusknorr
juliusknorr force-pushed the feat/version-comparison branch from 09745a2 to d19eaed Compare August 4, 2023 07:43
@juliusknorr

Copy link
Copy Markdown
Member Author

@jancborchardt @marcoambrosini Does it make sense to add images for side by side comparison? Videos/audio probably don't as per nextcloud/server#39140 (comment)

This would also give us at least one default handler we can use in server cypress tests for versions then. Otherwise I'd just cover this with tests in text.

@juliusknorr
juliusknorr force-pushed the feat/version-comparison branch from d19eaed to 8f598b3 Compare August 7, 2023 08:15
@juliusknorr
juliusknorr marked this pull request as ready for review August 7, 2023 08:15
@juliusknorr juliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 7, 2023
@marcoambrosini

Copy link
Copy Markdown
Member

@juliushaertl I don't think we have interesting use-cases for that, but I might be missing something :)

@juliusknorr
juliusknorr force-pushed the feat/version-comparison branch from 8f598b3 to 9d0e373 Compare August 7, 2023 10:33
@juliusknorr

Copy link
Copy Markdown
Member Author

Will push some tests in text then for this feature.

Signed-off-by: Julius H盲rtl <jus@bitgrid.net>
@juliusknorr
juliusknorr force-pushed the feat/version-comparison branch from 9d0e373 to 6b67e4e Compare August 8, 2023 13:09
@juliusknorr

Copy link
Copy Markdown
Member Author

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@skjnldsv

skjnldsv commented Aug 8, 2023

Copy link
Copy Markdown
Member

Cypress failure is also present on master, haven't found the time to adjust

@juliusknorr
juliusknorr merged commit ffd1c24 into master Aug 8, 2023
@juliusknorr
juliusknorr deleted the feat/version-comparison branch August 8, 2023 14:29
@skjnldsv

skjnldsv commented Aug 8, 2023

Copy link
Copy Markdown
Member

/backport to stable27

@backportbot-nextcloud backportbot-nextcloud Bot added the backport-request Pending backport by the backport-bot label Aug 8, 2023
@backportbot-nextcloud

Copy link
Copy Markdown

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@skjnldsv

skjnldsv commented Aug 8, 2023

Copy link
Copy Markdown
Member

/backport to stable27

@backportbot-nextcloud

Copy link
Copy Markdown

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@mejo-

mejo- commented Aug 8, 2023

Copy link
Copy Markdown
Member

/backport 6b67e4e to stable27

@backportbot-nextcloud

Copy link
Copy Markdown

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@juliusknorr

Copy link
Copy Markdown
Member Author

/backport 6b67e4e to stable27

@backportbot-nextcloud

Copy link
Copy Markdown

The backport to stable27 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable27
git pull origin stable27

# Create the new backport branch
git checkout -b fix/foo-stable27

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable27

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants