Skip to content

Conversation

@marcemarc
Copy link
Contributor

Description

What did you add/update/change?

When setting either the 'color' or the 'look' of the button for the external providers login button , although the settings come from the UUI link provided.. we are specifying the default values for the wrong thing in the list... so 'Color' can either be Default, Positive, Warning, Danger and
'Look' can be either Primary, Secondary, Outline or Placeholder

But we say here the default for color is 'secondary' and default for Look is 'default'

but they should be the other way around!!!

This PR flips them so they match the implementation

I've also added examples of what the colors are because, they are sort of historically bootstrappy names and not 'actual' colors :-P

Someone setting a property called 'color' might expect to be able to set it to a #hex reference or a color eg blue, but actually the options are not colors at all just words! I think although the info is 'there' in the UUI Colour link, it's not clear which one is which, bourne out by the fact that we've documented it wrong here, so maybe just listing the options as I've done here is helpful? but maybe less is more, and the link to the UUI library tells the full story!

Type of suggestion

  • Typo/grammar fix
  • Updated outdated content
  • New content
  • Updates related to a new version
  • Other

Product & version (if relevant)

Umbraco 15

Deadline (if relevant)

When should the content be published?

Now

When setting either the 'color' or the 'look' of the button for the external providers, although the settings come from the UUI link provided here... we are specifying the default values for the wrong list...
so 'Color' can either be Default, Positive, Warning, Danger
and
'Look' can be either Primary, Secondary, Outline or Placeholder

But we say here the default fo color is secondary and default for Looks is default

but they should be the other way around!
Copy link
Contributor

@sofietoft sofietoft left a comment

Choose a reason for hiding this comment

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

Thanks for PR @marcemarc !
Great to get this corrected, and it makes sense to add the different options as well.

I've made a few suggestions to the structure of it though - it might be a little easier to read when put into sub-bullets.
What do you think?

marcemarc and others added 2 commits January 9, 2025 16:23
@marcemarc
Copy link
Contributor Author

Thanks @sofietoft Thanks and Happy New Year... you know how I like one long sentence that never never ends, sometimes with commas but often just changing context in the middle, hope you had a great Christmas, but on reflection, I think:

yes, I like the bullets, so much easier to look up at a glance!

@sofietoft
Copy link
Contributor

Happy New Years to you as well @marcemarc - hope you had a wonderful holiday with the family!

And only 41 words? You can do better than that 😄 😉

Thanks for applying the suggestions here - I'll make sure the PR is merged.

@sofietoft sofietoft merged commit a2d2da4 into umbraco:main Jan 10, 2025
15 checks passed
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.

2 participants