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

File Tree Diff: small documentation to mention the file selector #12011

Merged
merged 4 commits into from
Feb 24, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions docs/user/visual-diff.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,30 @@ and focus your review on what has changed in the current page.

Example of Visual diff

Enabling Visual diff
--------------------
Using Visual diff
-----------------

Visual diff is enabled by default and it's only available on pull request builds.
Once enabled, a new UI element appears at the top-right of the page showing a dropdown selector containing
all the files that have changed between the base version (e.g. ``latest``) and the current pull request build.

You can select any of those files from the dropdown to jump directly into that page.
Once there, you can toggle it on/off by pressing the :guilabel:`Show diff` link from the UI element, or pressing the `d` key if you have hotkeys enabled.
Visual diff will show all the sections that have changed and their differences highlighted with red/green background colors.
Then you can jump between each of these chunks by clinking on the up/down arrows.

All the available configuration for the visual diff addon can be found under :guilabel:`Settings > Addons > Visual diff` in the :term:`dashboard`.
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if we could link to this directly with a URL arg 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Hrm, I think Sphinx doesn't allow us to use guilabel with a link: https://www.sphinx-doc.org/en/master/usage/restructuredtext/roles.html#role-guilabel

Do we have an example of what you would like to render? BTW, the link where would point to? I suppose we would need something like "Choose organization" feature but for projects.


Visual diff is only enabled on pull request builds,
and can be toggled on and off with the ``d`` hotkey.

Troubleshooting Visual diff
---------------------------

Visual diff only works if there are changes on the page,
Visual diff only works when we detect changes on the page,
so ensure you are on a page that has changed in the current pull request.

There are also some known issues that currently don't display properly:
There are also some known issues that currently don't display properly.
We are working to improve the UX, but so far we've found the following issues:

* **Tables** are shown to have changes when they may not have changed. This is due to do subtly in how HTML tables are rendered, and will be fixed in a future version.
* **Invisible changes** sometimes are marked as diff due than the underlying HTML changing, but there is no visual change. This could happen if the URL of a link changed, for example.
* **Chunks background is incorrect** when we are unable to detect the correct main parent element for the chunk.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% sure what this is saying? Will the background be missing or smaller than expected?

Copy link
Member Author

Choose a reason for hiding this comment

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

When we don't detect the immediate main parent element of the chunk, we keep going up to the parent of the parent and so on. It could happens that the highlighted node ends up being the whole document.

I tried to find a quick example, but I wasn't able to. However, I know this happens sometimes.

Copy link
Member

Choose a reason for hiding this comment

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

Seems like we just shouldn't highlight in that case? Maybe limit parent looking to 2-3 elements and explicitly avoid things that look like content divs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we will fix this issue. I tried a few different stuffs already but none worked reliably.

I think it's fine to document it as a known bug/issue for now.