Skip to content

Conversation

@Antoliny0919
Copy link
Member

@Antoliny0919 Antoliny0919 commented Aug 17, 2025

Trac ticket number

ticket-36554

Branch description

I fixed TabularInline to prevent link truncation when the title is long.

Before

Screenshot 2025-08-17 at 2 53 23 PM

After

Screenshot 2025-08-30 at 7 41 00 PM

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

Copy link
Member Author

@Antoliny0919 Antoliny0919 left a comment

Choose a reason for hiding this comment

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

I left a few comments regarding the design...

Comment on lines 288 to 290
.inline-group .tabular tr td.original a + a {
margin-right: 8px;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I added the above CSS to divide the links.
Honestly, I’d like to avoid adding CSS specifically for RTL whenever possible.

Screenshot 2025-08-17 at 3 02 16 PM

I’m not sure if it’s just me, but when both links exist in Tabular or Stacked, they’re too close together, making them a bit hard to distinguish.
(It looks like a single link.)

Is there a good way to distinguish the two links when both are present?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think given that in other places, this is a view on site button, we should make it a view on site button with the same formatting:
Screenshot from 2025-08-28 13-32-37

@sarahboyce sarahboyce added selenium Apply to have Selenium tests run on a PR screenshots 🖼️ labels Aug 27, 2025
Copy link
Contributor

@sarahboyce sarahboyce left a comment

Choose a reason for hiding this comment

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

You need to check the selenium tests but I found that with this change, I can no longer create new items in the tabular inline (I can't update the fields or delete the row entries)

Comment on lines 288 to 290
.inline-group .tabular tr td.original a + a {
margin-right: 8px;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think given that in other places, this is a view on site button, we should make it a view on site button with the same formatting:
Screenshot from 2025-08-28 13-32-37

@Antoliny0919
Copy link
Member Author

Antoliny0919 commented Aug 30, 2025

You need to check the selenium tests but I found that with this change, I can no longer create new items in the tabular inline (I can't update the fields or delete the row entries)

Thank you Sarah, Looks like the absolute positioned element is covering the interactive buttons. 🥲

@Antoliny0919 Antoliny0919 force-pushed the ticket_36554 branch 2 times, most recently from 569f88a to b87d22e Compare August 30, 2025 10:35
@sarahboyce sarahboyce force-pushed the ticket_36554 branch 2 times, most recently from 742f4c5 to 6344c1b Compare September 22, 2025 07:55
@sarahboyce
Copy link
Contributor

@Antoliny0919 can you take a look at the "View on Site" button for a stackedinline on mobile/tablet views:

Screenshot from 2025-09-22 10-06-23

I think it would be better if either both the change link and view on site button is on a new line or they are together

@Antoliny0919 Antoliny0919 force-pushed the ticket_36554 branch 2 times, most recently from b733643 to 30d6ef8 Compare September 23, 2025 12:38
@Antoliny0919
Copy link
Member Author

@Antoliny0919 can you take a look at the "View on Site" button for a stackedinline on mobile/tablet views:
I think it would be better if either both the change link and view on site button is on a new line or they are together

Thank you Sarah :) I agree. It would be better to separate the links and the object string into different lines.
By the way, the image above wasn’t generated through screenshot tests, right?
I didn’t see it when I was testing.
How about adding a screenshot test for StackedInline when an object exist? (+ links)
It seems like we don’t have one yet.

@sarahboyce
Copy link
Contributor

By the way, the image above wasn’t generated through screenshot tests, right?

Correct, I found this through manual testing. Having a screenshot with the StackedInline is also a good idea.
I would have the changes around this link becoming a button, including the additional test, be in a separate commit if possible

@sarahboyce
Copy link
Contributor

Screenshot from 2025-09-26 15-42-58

So now the buttons are always on a new line in the staked case, which I'm not sure this is what's wanted

In the ticket (and I know it's discussed in the forum: https://forum.djangoproject.com/t/improving-consistency-and-accessibility-between-tabularinline-and-stackedinline/37951/2), I was hoping we'd land on having the stackedinline and taburlarinline having the same design approach. Currently, this PR makes the two designs diverge further
Perhaps we should remove the model name prefix from the stacked inlines but otherwise use the stacked inline design for the titles? So then we don't truncate the name of the tabular inlines?

@Antoliny0919
Copy link
Member Author

Antoliny0919 commented Sep 26, 2025

So now the buttons are always on a new line in the staked case, which I'm not sure this is what's wanted

In the ticket (and I know it's discussed in the forum: https://forum.djangoproject.com/t/improving-consistency-and-accessibility-between-tabularinline-and-stackedinline/37951/2), I was hoping we'd land on having the stackedinline and taburlarinline having the same design approach. Currently, this PR makes the two designs diverge further Perhaps we should remove the model name prefix from the stacked inlines but otherwise use the stacked inline design for the titles? So then we don't truncate the name of the tabular inlines?

I think that would probably be the most ideal.
But since TabularInline is in a table format and the object __str__ is displayed in a single column.
hmm… I guess I’ll have to give it a try.
Thanks for the great suggestion Sarah!

@Antoliny0919
Copy link
Member Author

tabular_group

In the Tabular layout, a single object can be represented using up to three rows.
Do you think it would be better to unify the background color of these three rows(row group)?

Previously, a single object could take up to two rows (error, data), and in that case, the background colors were not unified.

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

Labels

screenshots 🖼️ selenium Apply to have Selenium tests run on a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants