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

Fix first and last action input padding #1967

Merged
merged 1 commit into from
May 18, 2021

Conversation

raimund-schluessler
Copy link
Contributor

@raimund-schluessler raimund-schluessler commented May 17, 2021

This fixes the padding of the ActionInput in case it is the first or last entry. The currently set margin lead to a gap between the popover arrow and the input element.

Before:
Screenshot_2021-05-17 Nextcloud Vue Style Guide(2)

After:
Screenshot_2021-05-17 Nextcloud Vue Style Guide(3)

In case one switches from ActionButton to ActionInput like it happens in Calendar, this will still lead to a "jumping" of the popover menu, since the ActionButton has a different height than the ActionInput with the additional padding (see nextcloud/calendar#3016). But the gap is not there anymore.

actioninput

On a side note: I would say the additional padding is not really necessary (or at least to high). But since this is supposed to be a bug fix PR, I didn't remove it, but instead made it work properly 😉. @jancborchardt maybe to check if this is really what it is supposed to look like.

Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler raimund-schluessler added 3. to review Waiting for reviews bug Something isn't working design Design, UX, interface and interaction design feature: actions Related to the actions components labels May 17, 2021
Copy link
Contributor

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Fine by me for a bugfix

On a side note: I would say the additional padding is not really necessary (or at least to high). But since this is supposed to be a bug fix PR, I didn't remove it, but instead made it work properly wink. @jancborchardt maybe to check if this is really what it is supposed to look like.

I agree, either that or we need to make sure the spacing applies to all elements. The only thing where this might be odd is with the hover/focus feedback of the ActionButton, which if we increase just the top padding would look rather unaligned with the background then.

@raimund-schluessler
Copy link
Contributor Author

I agree, either that or we need to make sure the spacing applies to all elements. The only thing where this might be odd is with the hover/focus feedback of the ActionButton, which if we increase just the top padding would look rather unaligned with the background then.

Not sure if it's a good idea to apply it to all elements. This will look really unaligned.

@raimund-schluessler raimund-schluessler merged commit fade1dc into master May 18, 2021
@raimund-schluessler raimund-schluessler deleted the fix/noid/action-input-padding branch May 18, 2021 08:22
@skjnldsv skjnldsv mentioned this pull request Jun 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working design Design, UX, interface and interaction design feature: actions Related to the actions components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants