Skip to content
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

WIP: Add new draft styles #4030

Merged
merged 6 commits into from
Jul 25, 2024
Merged

WIP: Add new draft styles #4030

merged 6 commits into from
Jul 25, 2024

Conversation

nicodh
Copy link
Contributor

@nicodh nicodh commented Jul 12, 2024

resolves #3968

@nicodh
Copy link
Contributor Author

nicodh commented Jul 12, 2024

TBD: should we scroll down when the attachement draft is added?

Styling is just a proposal - reduced opcity should show the "draft" state

Draft Image
image

VCard
image

Datei
image

@nicodh nicodh requested review from Simon-Laux and r10s July 12, 2024 23:17
@r10s
Copy link
Member

r10s commented Jul 12, 2024

looks nice, thanks a lot for taking care!

at a first glance, however, i think the old "white outer box" makes things more clear: that the bubble is not sent yet, what the staging area is, when quotes are added etc. still, inside the white box, the nicely styled card/image can be shown.

but i am afk, only looking at the screenshots, i can have a closer look tomorrow.

@Simon-Laux
Copy link
Member

My ideas on this:

  • keep in in the composer (white background), I agree with r10s on that point
  • change to 100% opacity
  • use some subtle color as background that you can still see the rounded corners, but the color is subtle that it still looks good

Copy link
Member

@r10s r10s left a comment

Choose a reason for hiding this comment

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

i reviewed in a little more detail and played around with things on my desktop.

scss/composer/_composer.scss Show resolved Hide resolved
scss/composer/_composer.scss Outdated Show resolved Hide resolved
Comment on lines 40 to 41
padding-left: 8px;
margin-bottom: 12px;
Copy link
Member

Choose a reason for hiding this comment

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

the padding/margin here could be removed again when going for the general "padding: 9px;" suggestion above.

having simple, horizontally symmetric spacings is also better wrt to the rtl issue that needs to be targeted at some point, btw

@nicodh nicodh requested a review from r10s July 18, 2024 14:40
@nicodh
Copy link
Contributor Author

nicodh commented Jul 18, 2024

Image

image

VCard

image

Datei

image

Next proposal...

@nicodh nicodh removed the request for review from Simon-Laux July 25, 2024 08:03
Copy link
Member

@r10s r10s left a comment

Choose a reason for hiding this comment

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

tested - and it is a huge improvement that can be merged :)

also, the "close" button and its hover-effect is much nicer now (as no longer a long, vertical bar when hovered)

what i would streamline is the different position of the "close" button - for images/quotes is it on the right edge - for contacts it is directly after the contact card.

i think, the old approach, "right edge" is better as there are often combinations with quotes and we have two "X" - and it looks much nicer and is better usable if they're at the same position.

so, i would change that bit for the contact-cards/files as well (so moving the "x" to the edge). but that is no blocker :)

@nicodh
Copy link
Contributor Author

nicodh commented Jul 25, 2024

That how I implemented it before, and Simon asked me to keep the icon nearer to the VCard & File since on bigger screen the X is pretty far from the left aligned VCard :-)

But I agree if there are multiple delete icons it looks strange if they are positioned different

@nicodh nicodh merged commit e83350b into main Jul 25, 2024
5 of 7 checks passed
@nicodh nicodh deleted the polish-composer-draft branch July 25, 2024 20:49
@r10s
Copy link
Member

r10s commented Jul 25, 2024

for the distance on bigger screens: yes, that is true, thanks for bringing that up. however, this is how it usually is, ppl are used to it. moreover, in this case, deletion is less a things than eg. for closing dialog. ppl just searched and added for a card/file/image, it is not the most common case to delete that immediately again, so a clearer, less cluttered UI seems advantageous here.
finally, also the send button is at the same horizontal offset, so, the "X" are not out of view at all, it is even sort of a "focus column" where you can "do things". just some additional thoughts :)

EDIT: just tested, it looks great! thanks a lot for taking care!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vcard staging polishing needed
3 participants