Fixed #34917 -- Underlined the 'a' tags in the main content area of the admin. #18958
Conversation
|
The added underline part... Login
Logout
Password Reset Done
List Display
Filter
Add/Change Form Fields
Tabular/Stacked Inline
Message
Breadcrumbs
Recent Actions
|
dbacdab to
d5108bb
Compare
|
Could you give me some suggest on a few things? 🧐
|
d5108bb to
608a819
Compare
There was a problem hiding this comment.
Thank you @Antoliny0919
I've added a suggestion which I hope will address your questions above 👍
3cecb5e to
bd79d5e
Compare
|
Here is the part where the underline has been added. Please let me know if anything is missing or unnecessary! |
Should we also add an underline to the message? For now, I’ve left it without one. |
|
I think underlining links in messages would be good, especially as the contrast could be a bit lacking depending on the background colour for each message level |
I agree as well. Since there is a background color, I feel like this might be the most necessary part. |
bd79d5e to
9a7327c
Compare
| @@ -887,6 +891,10 @@ a.deletelink:focus, a.deletelink:hover { | |||
| width: 100%; | |||
| } | |||
|
|
|||
| #content-main:not(.app-list) a:not([role="button"]) { | |||
There was a problem hiding this comment.
| #content-main:not(.app-list) a:not([role="button"]) { | |
| a:not(#nav-sidebar a):not(#content-main.app-list a):not([role="button"]) { |
I think maybe we can do this and then remove the other css rule and the <div id="content-main"> additions.
Part of the reason is I feel like the breadcrumbs and the recent actions section should also have underlines 🤔
After that, I think we're just missing a review from the accessibility team and we need to update the screenshots of the admin in the docs
There was a problem hiding this comment.
Thank you for the great suggestion, @sarahboyce 😃
Hmm... I'm not sure if breadcrumbs really need underlines. Since breadcrumbs are already designed in a very intuitive way, I believe users can easily recognize them as links without the need for underlines.
(Considering that the target users are developers with some knowledge of the web...)
Or maybe it would be good to add a --header-link-color underline to the breadcrumbs, similar to the one applied to user-tools.
For now, I'll follow your suggestion and add underlines to both the breadcrumbs and recent actions.
Additionally, links are also being added to the header, which I assume was unexpected.
So, I'll exclude the links in the header and apply the following selectors instead.
a:not(
[role="button"],
#header a,
#nav-sidebar a,
#content-main.app-list a
) {
text-decoration: underline;
}
There was a problem hiding this comment.
I don't have a strong opinion on the breadcumb underlines - maybe I'd weakly prefer it.
But one thing to keep in mind is that the admin isn't only used by developers, it's also used a lot by customer support, operations and all sorts of people in early stage (and later) companies that haven't built their own admin yet (or never will), and I've also seen this with agencies giving the site owner who knows more or less nothing access and a bit of training to make changes to the site.
c2fadf2 to
71127e2
Compare
53f4bf9 to
ad4b62c
Compare
|
The calendar icon button is no longer visible on dark mode |
docs/ref/contrib/admin/_images/actions-as-modeladmin-methods.png
Outdated
Show resolved
Hide resolved
ad4b62c to
3427f99
Compare
There was a problem hiding this comment.
Thank you for the screenshot updates @Antoliny0919 😍
Love that the sizes are much more consistent
Going through the visual regression screenshots and it looks like when the page is first loaded the "Add another ..." button has an underline but then it goes away 🤔
docs/intro/_images/admin02.png
Outdated
There was a problem hiding this comment.
Quoting Tom from the ticket
I think this should ideally exclude the site index and app index pages (they're quite clear), but acceptable if these are underlined as well.
What do we think? These have underlines here but no underlines when within the sidebar
There was a problem hiding this comment.
Are you referring to the recent actions section in the sidebar?
From what I can see in the image, there don’t seem to be any actions related to change or additions, so no links are displayed. But if there were such actions, I believe links with underlines would appear.
I think this should ideally exclude the site index and app index pages (they're quite clear), but acceptable if these are underlined as well.
This part feels quite challenging...
Thinking back to the discussions we had on Discord about this change, underlines were a feature that people had mixed opinions about.
Personally, I also think the index page and app index page elements are clear enough without underlines (though this might just be my personal opinion).
So, even if it slightly affects consistency, I’d lean towards excluding underlines from the links on the index and app index pages.
There was a problem hiding this comment.
Actually I think what you have code wise is correct, but this screenshot is wrong. I don't think you need to update this screenshot at all
There was a problem hiding this comment.
Sorry about that. I was confused as well 🥲
docs/intro/_images/admin03t.png
Outdated
There was a problem hiding this comment.
This is also outdated, I think we can revert this change
3427f99 to
44b9a57
Compare
44b9a57 to
e92764c
Compare
e92764c to
8623e70
Compare
Trac ticket number
ticket-34917
Branch description
Continue @Myrausman's PR
Add underline to "a" tags "main content area" in the Django admin.
Checklist
mainbranch.