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: Clean up chip list spacing #164

Merged
merged 4 commits into from
Feb 14, 2024
Merged

fix: Clean up chip list spacing #164

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b6a7abe
Clean up chip list spacing
vinnyjth Feb 8, 2024
b463c44
Update packages/web-shared/components/Searchbar/Search.styles.js
vinnyjth Feb 13, 2024
abc36e9
Better margin + padding on scroll bar
vinnyjth Feb 13, 2024
1bdeb9d
Merge branch 'main' into chip-list-spacing
nlewis84 Feb 14, 2024
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
3 changes: 1 addition & 2 deletions packages/web-shared/components/ContentSingle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@ function ContentSingle(props = {}) {
</BodyText>
);


return (
<>
{/* TODO: Max width set to 750px due to low resolution pictures. Can be increased as higher quality images are used */}
Expand Down Expand Up @@ -241,7 +240,7 @@ function ContentSingle(props = {}) {
</Box>
{/* Sub-Feature Feed */}
{hasFeatures ? (
<Box my="l">
<Box mb="l">
<FeatureFeed data={feed} />
</Box>
) : null}
Expand Down
10 changes: 7 additions & 3 deletions packages/web-shared/components/FeatureFeed/Features/ChipListFeature.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,15 @@ function ChipListFeature(props = {}) {
return null;
}

const title = props.feature.title || props.feature.subtitle;

return (
<Box className="chip-list-feature">
<Box padding="xs" fontWeight="600" color="base.gray" id="results">
{props.feature.title || props.feature.subtitle}
</Box>
{!!title ? (
<Box padding="xs" fontWeight="600" color="base.gray" id="results">
{title}
</Box>
) : null}
<Styled.List>
{props.feature?.chips?.map(({ title, iconName, relatedNode }, index) => {
return (
Expand Down
1 change: 0 additions & 1 deletion packages/web-shared/components/FeatureFeed/Features/ChipListFeature.styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ const Chip = withTheme(styled.a`
const List = withTheme(styled.ul`
display: flex;
align-items: center;
overflow-x: scroll;
width: 100%;
gap: 8px;
// Reset ul margin
Expand Down
10 changes: 6 additions & 4 deletions packages/web-shared/components/Searchbar/Search.styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,14 @@ const Wrapper = withTheme(styled.div`
border-radius: 0;
}

.chip-list-feature {
* > &:first-child {
.chip-list-feature ul {
overflow-x: scroll;
vinnyjth marked this conversation as resolved.
Show resolved Hide resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

I think adding padding-bottom: 1rem; here might be nice.

Copy link
Member Author

Choose a reason for hiding this comment

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

👁️ I'll take a look!

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks good without the scrollbar, looks not so good with the scrollbar. Margin might look great.
CleanShot 2024-02-13 at 12 57 46@2x

Copy link
Member Author

Choose a reason for hiding this comment

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

Splitting 1rem across margin + padding

CleanShot 2024-02-13 at 12 59 01@2x

Just margin

CleanShot 2024-02-13 at 12 59 11@2x

I think Iike splitting the most

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah Splitting works for me

& > *:first-child {
vinnyjth marked this conversation as resolved.
Show resolved Hide resolved
margin-left: 1rem;
}
* > &:last-child {
margin-left: 1rem;
& > *:last-child {
Copy link
Member Author

Choose a reason for hiding this comment

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

I wrote this CSS in the previous PR, had it working, then moved it to a different section and butchered it when re-writing 😅

margin-right: 1rem;
}
}
`);
Expand Down
3 changes: 2 additions & 1 deletion web-embeds/public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@
style="max-width: 1180px; padding: 40px; margin: auto; margin-top: 20px"
></div> -->
<div data-church="liquid_church" data-type="Auth" data-modal="true" class="apollos-widget"
style="max-width: 1180px; padding: 40px; margin: auto; margin-top: 20px"></div>
style="max-width: 1180px; padding: 40px; margin: auto; margin-top: 20px"
data-search-feed="FeatureFeed:07e1050d-a6c4-4b46-a2a0-12d8fbf1df3c"></div>
<div data-type="FeatureFeed" data-church="liquid_church"
data-feature-feed="FeatureFeed:d3912726-487b-4635-9cec-99ed84a21209" data-modal="true" class="apollos-widget"></div>

Expand Down
Loading