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

New message form facelift #4584

Merged
merged 3 commits into from
Jan 8, 2021

Conversation

marcoambrosini
Copy link
Member

Peek 2020-11-12 14-27

Signed-off-by: Marco Ambrosini marcoambrosini@pm.me

@marcoambrosini
Copy link
Member Author

marcoambrosini commented Nov 13, 2020

@nickvergessen pointed out that there are too many shades of gray. I could remove the circle around the X button when it's not hovered so it's bee one less shade. What do you think?

@jancborchardt
Copy link
Member

As briefly mentioned in our call, I would say if we change it, then I’d expect the input field to be grey, not the block around it.

Because the current style (please always do before / after comparison screenshots :) actually looks very light and nice like @jenniferpiperek designed it, and changing to having a whole grey block down there is a bit massive. (As @nickvergessen also pointed out.)

(And then again, input fields are usually not grey but always white with a border around it, so I see a high risk in breaking that convention. On first glance it would look like a disabled button or disabled input field more than a regular input field.)

@marcoambrosini
Copy link
Member Author

input fields are usually not grey but always white with a border around it, so I see a high risk in breaking that convention.

Pushed a second attempt with border on the input field

Peek 2020-12-18 13-13

@jancborchardt
Copy link
Member

Looks better, and then we don't need the additional horizontal line anymore? Could either be done with a fade to white, or only showing the line when not scrolled all the way to the bottom (that's what e.g. Signal does).

@marcoambrosini
Copy link
Member Author

Done it the signal way, looks pretty darn good to me!

Peek 2020-12-29 11-36

@jancborchardt
Copy link
Member

Looking great now! :) The commits seem messed up though, as e.g. also the button-looking date dividers which we don’t want are in here?

@marcoambrosini
Copy link
Member Author

marcoambrosini commented Dec 30, 2020

Looking great now! :) The commits seem messed up though, as e.g. also the button-looking date dividers which we don’t want are in here?

Yep needs some cleanup.
This branch Is based on the date-buttons branch because of the messagelist alignment issues fixed there. We don't want them? I thought that the discussion was still open, please see the updates!
I think that they look very nice and that there's no real danger that someone would mistake them for buttons, but if you still are not convinced, let's at least merge the center alignment of the system messages and the layour fixes there.

@marcoambrosini marcoambrosini force-pushed the feature/noid/new-message-form-facelift branch from bafa13f to d47ff3e Compare January 7, 2021 11:08
@marcoambrosini marcoambrosini force-pushed the feature/noid/new-message-form-facelift branch 2 times, most recently from 97a2824 to cf4d006 Compare January 7, 2021 11:19
Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
@marcoambrosini marcoambrosini force-pushed the feature/noid/new-message-form-facelift branch from cf4d006 to 2becc41 Compare January 7, 2021 13:59
class="new-message-form__button submit"
@click.prevent="handleSubmit">
<Send
:size="20"
Copy link
Member

Choose a reason for hiding this comment

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

no title, so its always "Send" instead of being translated

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
@nickvergessen nickvergessen merged commit 252f871 into master Jan 8, 2021
@nickvergessen nickvergessen deleted the feature/noid/new-message-form-facelift branch January 8, 2021 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants