-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Refine files comments design #30412
Refine files comments design #30412
Conversation
Signed-off-by: Quentin Guidée <[email protected]>
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.
I really like how it tidies up things, but I would loose the gray background and play with white space instead to separate the elements
Yup, agree with that. :) Otherwise the box separates the comment body text too much from the commenter and timestamp. Also we never use containers like that either in e.g. Talk. Also, the indentation of the text is intentional (and as usually chat apps with avatars on the side do it), without that it looks quite crowded. |
Yeah I recognised that and tried to keep it, but I think we're facing a problem here. If the text is aligned with the username like many apps, I think the message should be placed higher ("in the header") : I think it looks way better like this, but the problem is that NC want 44px height buttons, so the "…" will looks totally off. |
Exactly, that looks great @quentinguidee! :) (Only one thign: No all-caps in the timestamp as per our design ghideliens for readability.) @marcoambrosini how do you think we could account for this? |
I actually think it makes a lot of sense to have the paragraph span the whole width in the sidebar, given the very limited width of the sidebar itself. I've been thinking about doing the same exact thing in talk's chat sidebar. If we do that there's going to be more space for the text anyway and we can afford a little bit more vertical whitespace if separation is still a concern |
Not sure about that – I was actually going to say that it would be good if comments and Talk (chat in sidebar) simply uses the same component, but with the indentation, as it is in both Talk and Comments atm. The width is completely fine, it’s a similar form factor as e.g. for document comments in Office. |
@quentinguidee your last version looked really nice, is there a follow-up pull request or how come you closed this one? :) |
That was not planned as I thought there was not a big interest around this PR, but I'll reopen one. Edit: #35870 |
Motivation
This PR
Screenshots
Before/after
Before/after (video)
Enregistrement.de.l.ecran.2021-12-27.a.11.35.36.mov