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.

Sign up for GitHub

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: added text and changed margins #12923

Merged
merged 9 commits into from
Feb 8, 2021
+11 −0
Merged

fix: added text and changed margins #12923

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
11fcd58
added text and changed margins
AAfghahi Feb 3, 2021
6ee5f8e
linted
AAfghahi Feb 3, 2021
57e055f
changed the margin-bottom only for DatasourceEditor
AAfghahi Feb 3, 2021
6f2d420
help block instead of form group margin
AAfghahi Feb 4, 2021
ea868cc
whoops meant margin-top
AAfghahi Feb 4, 2021
07d05ed
removed floating x
AAfghahi Feb 4, 2021
069f55c
Merge branch 'master' into update-text
AAfghahi Feb 5, 2021
4a5441c
changed margins
AAfghahi Feb 5, 2021
3c9c4b6
actual changes
AAfghahi Feb 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
changed margins
@AAfghahi
AAfghahi committed Feb 5, 2021
commit 4a5441c0713541b7f49fda1ef0327fb569deefbc
4 changes: 2 additions & 2 deletions superset-frontend/src/datasource/DatasourceEditor.jsx
View file
Original file line number Diff line number Diff line change
@@ -59,8 +59,8 @@ const DatasourceContainer = styled.div`
font-weight: ${({ theme }) => theme.typography.weights.bold};
}

.help-block {
margin-top: 8px;
.form-group.has-feedback > .help-block {
margin-top: -8px;
}
Copy link
Member

Choose a reason for hiding this comment

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

when I render this, I still see the margin-bottom of the form-group, so it's still making a total of 16px.
This is a bit tricky actually. I read this as

form-group should always have a margin bottom of 8px
when there are two siblings (form-group followed by a form-group), then the second one should have a margin-top of 16px.

I think if you go that way, you won't need to add anything additional to the help-block.
lmk if that helps

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that helps a lot, it really cleared up my thinking about it. I also ended up reading more about how margin works. I ended up trying something else, because nothing I was reading was helping me find a way of implementing your solution.

I ended up targeting the specific form group that is above each help block (form-group-md). I changed the margin-bottom of these to be 8px. However, in order to make sure that 'Extra' and 'Owners' section had a help block with a margin of 8px, I also added a margin-top to the help-block.

Originally I thought that a margin-bottom of 8 and a margin-top of 8 on two elements would make 16px, but they don't actually add the way that I thought they might. It simple goes to the highest margin value. Which is cool.

It looks like this now:
image

The margin between form groups is 16px:
image

The margin between help-block and form-group is 8px:
image

Copy link
Member

Choose a reason for hiding this comment

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

This look great. One other thing I was hinting at was sibling selectors: when there are two siblings (form-group followed by a form-group), then the second one should have a margin-top of 16px. So that would be .form-group + .form-group but it looks like you made it work this way, too.

`;