-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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`. | ||
|
||
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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
There was a problem hiding this comment.
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 🤔
There was a problem hiding this comment.
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.