Conversation
Codecov Report
@@ Coverage Diff @@
## master #71 +/- ##
=======================================
Coverage 68.34% 68.34%
=======================================
Files 66 66
Lines 1055 1055
Branches 177 177
=======================================
Hits 721 721
Misses 330 330
Partials 4 4Continue to review full report at Codecov.
|
|
|
||
| Types cannot be used simultaneously on the same element. For example, "cats" and "dogs" are types of animals, but are mutually exclusive. | ||
|
|
||
| <b>Comment</b>: We should use types for defining the standard appearance of the component. What we have so far is using the primary and secondary as types, which in my opinion are variations of the component that are only between themselves mutually exclusive. So, we can define those as enum variation of the component, but sadly we so far come up with the name 'type' for the prop that can be confusing. I propose we change the name of this prop with something like suiStyle (bootstrap has something similar – bsStyle). Should we add here the positive and negative props as well? After this we can use one prop with enum for defining all different types of the component (similar in a way of the shape property in the current [Menu component](https://github.com/stardust-ui/stardust/pull/68) |
There was a problem hiding this comment.
I added here comment about whether we should replace the type property with something like suiStyle, for setting the primary, secondary, positive or negative variations of the components. Now that we have introduced the shape property, we can still stick with the type property as initially. We should decide on this and I will change the description accordingly.
There was a problem hiding this comment.
my opinion on that is that type is a much pleasant name in contrast to suiStyle - while the later one seems more specific, the problem is that it is not as intuitive for the client as the first one. It would be really hard for the client to remember it in a way to not use docs as a refresher
There was a problem hiding this comment.
with that being said, would rather suggest to apply primary and secondary as props, even given that they do not fit common logic so far. Another option is to come up with another property name that is not library specific (is close to the general notion), like kind or something like that
There was a problem hiding this comment.
I am definitely for having one prop with different values for the primary, secondary, positive and negative, as a hint to the user that these cannot be applied at the same time. And yes, I agree that we should come up with a good name, as this is one of the props which will be most commonly used. @levithomason any thoughts on this?
There was a problem hiding this comment.
I propose we merge this PR as is now. In the glossary table this property is described as type, which is the current implementation, and we can update it later if we think it would be appropriate. All other PRs should update this table if they are introducing new props, and we still don't have this merged on the master which can become a problem. @kuzhelov ?
There was a problem hiding this comment.
agree, it could work as long as we won't introduce another possible types for components (that could be applied at the same time with primary or secondary). At this point we could rethink this taken approach
| fixed | Used to make the component fixed to a side of it's content. ('left', 'right', 'top', 'bottom') | ||
| textAlign | Align the content. ('left', 'right', 'center', 'justified') | ||
| vertical | Used to make the component display it's content vertically. | ||
| shape | Used to describe the shape of the component. For example for the Menu component, beside the default, we have the following shapes: ('pills', 'pointing', 'underlined') |
There was a problem hiding this comment.
I'm adding a new property to the buttons that have icons, called iconPosition; it can
-
description line that I'm proposing:
+iconPosition | Used to describe if an icon is positioned to the left or to the right of the main component. Default value is 'left' ('left', 'right') -
example of usage in
Buttoncomponent:
<Button icon="book" iconPosition="right" content="click" />
levithomason
left a comment
levithomason
left a comment
There was a problem hiding this comment.
Let's merge this and iterate on it in more focused PRs. This is a good baseline to get us started.
|
|
4 participants
Added initial glossary specification. We can split this later into multiple files.