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

Add a button style for being used on primary like in the header #2871

Merged
merged 1 commit into from
Jul 22, 2022

Conversation

nickvergessen
Copy link
Contributor

@nickvergessen nickvergessen commented Jul 21, 2022

Bildschirmfoto vom 2022-07-21 16-14-07

Fix #2869

Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen added bug Something isn't working 3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities feature: button labels Jul 21, 2022
@nickvergessen
Copy link
Contributor Author

And yes, the action menu is following the wrong color there as it is not using the vue component yet

@@ -595,6 +600,18 @@ export default {
}
}

// Tertiary on primary color (like the header)
&--vue-tertiary-on-primary {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just call it "inverted colours" or something like that? I can imagine we could use something similar in talk's call view

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No that's not how it works. In Talk we would always need white or something alike, not depening on the primary color nor the background.

@@ -535,6 +535,11 @@ export default {
&.button-vue--vue-tertiary-no-background {
opacity: 1;
}
&.button-vue--vue-tertiary-on-primary {
border-radius: var(--border-radius);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
border-radius: var(--border-radius);
border-radius: var(--border-radius-large);

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we do this via a separate prop square? I'm using this for the input too. Sometimes it's needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes but we need this kind of buttons elsewhere too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the square (if we introduce it) should work on all buttons, so I would like to have that done in a seperate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reported at #2873

@marcoambrosini marcoambrosini merged commit 8390079 into master Jul 22, 2022
@marcoambrosini marcoambrosini deleted the bugfix/2869/button-teriary-on-primary branch July 22, 2022 08:07
@danxuliu
Copy link
Contributor

And yes, the action menu is following the wrong color there as it is not using the vue component yet

For reference: nextcloud/server#33229

Independently of that, I have noticed that in the :active state (which can be triggered by pressing on the button and then moving the cursor away so it no longer hovers it, which overrides :active) a round background is shown for the button, even if the box shadow is (almost) square when focused. Is it known / expected? Or should I open an issue?
Tertiary-On-Primary-Active

@nickvergessen
Copy link
Contributor Author

Not expected, please raise an issue

@nickvergessen
Copy link
Contributor Author

/backport to stable5

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 accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working feature: button
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Button style for a button on top of primary color
4 participants