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 card shrink in Catalog #1650

Merged
merged 2 commits into from
Apr 13, 2020
Merged

Fix card shrink in Catalog #1650

merged 2 commits into from
Apr 13, 2020

Conversation

andresmgot
Copy link
Contributor

Description of the change

When there are less than 4 cards in the catalog, these cards shrink

Screenshot from 2020-04-08 17-32-51

We need to set the CSS properties to fill the whole row.

Screenshot from 2020-04-08 17-40-11

@@ -143,7 +143,7 @@ class Catalog extends React.Component<ICatalogProps, ICatalogState> {
</div>
</div>
)}
<div className={this.shouldRenderOperators() ? "col-10" : ""}>
<div className={this.shouldRenderOperators() ? "col-10" : "col-12"}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this dependent on whether we're rendering operators? That is, why not use the same width ("col-10") either way? If it's because the filter is not present, then we should use a function that makes that obvious, perhaps?

Copy link
Contributor Author

@andresmgot andresmgot Apr 13, 2020

Choose a reason for hiding this comment

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

yes, it's because if there are no operators we are skipping the column that allows you to filter by chart/operator. This function is used several times (line 102 and 121), I guess it's more obvious in the full context of the function.

@andresmgot andresmgot merged commit aeccc10 into master Apr 13, 2020
@andresmgot andresmgot deleted the catalogShrink branch April 13, 2020 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants