File Tree Diff: updates to UI (info and PR link)

[humitos]

This is what I understood we agreed on in our meetting.

Peek.2025-02-20.11-45.webm

• Closes Visual diff: updates to the UI #549
• Closes Remove PR build notification and add info to Visual Diff #548

ToDo

• add tests
• require build URL in data validation

[ericholscher]

Yea, this is way nicer 💯

It doesn't remove the PR build notification tho :D

[humitos]

It doesn't remove the PR build notification tho :D

Haha, that was on purpose. I didn't want to make that part of this change. However, I think #552 is a better solution.

[ericholscher]

It doesn't remove the PR build notification tho :D

Haha, that was on purpose. I didn't want to make that part of this change. However, I think #552 is a better solution.

Only if we have it off by default? I really think avoiding notifications unless totally necessary is the right UX. The visual diff UI is a much better way to communicate that info.

[humitos]

Only if we have it off by default? I really think avoiding notifications unless totally necessary is the right UX. The visual diff UI is a much better way to communicate that info.

I can open another PR for this change but I need to understand what's the exact behavior you want here first.

Do you want to remove the notification " This page was created from a pull request build"...

• completely/always?
• only when file tree diff is enabled?
• even if we merge Notification: render it inline using docTool.getRootSelector() #552?
• etc...

If you describe all the cases for the expected UX I can put a PR with that.

[humitos]

Also, take into account that we have a setting in the WebUI: https://github.com/readthedocs/readthedocs.org/blob/29a912f4aad30b83b29539cf4ab46c5f7721f040/readthedocs/projects/models.py#L262

that controls whether or not show notification on external versions. I'm not sure we want to ignore that setting.

[humitos]

Summarizing, I don't want to change the current behavior without considering all of that -- but I'm happy to talk and change the current behavior if it's not what we want.

[ericholscher]

I want inline PR build notifications that are off by default, but can be enabled in the dashboard with that model field.

[humitos]

I want inline PR build notifications that are off by default, but can be enabled in the dashboard with that model field.

OK. That would need #552 and also to change our current default value in .org.

I will merge this PR and we can continue discussing that in another issue. I opened #557 for this.