Skip to content

Add Overflow to prop aliases #128

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

Closed
innerdaze opened this issue Mar 12, 2019 · 5 comments
Closed

Add Overflow to prop aliases #128

innerdaze opened this issue Mar 12, 2019 · 5 comments

Comments

@innerdaze
Copy link

innerdaze commented Mar 12, 2019

Why:

Consider this common mobile layout:

const template = `
  header
  content
  footer
`
const C = () => (
  <Composition
    template={template}
    templateRows='auto 1fr auto'
    height='100vh'>
    {({ Header, Content, Footer }) => (
      <>
        <Header/>
        <Content/>
        <Footer/>
      </>
    )}
  </Composition>
)

Now say we want the Content area to be vertically scrollable - this means that the Content div needs to have overflow-y: scroll in it's style.

Without top-level support for overflow we would need to apply the style inside the FaaC.

Why I think this is a good candidate:
Quite simply - I believe scroll is a layout concern; it's not styling.

How:

Add the following prop aliases to the Box component.

alias: overflowY
type: string
mapping: {
  undefined: `overflow-y: auto`
  [value]: `overflow-y: ${value}`
}

alias: overflowX
type: string
mapping: {
  undefined: `overflow-x: auto`
  [value]: `overflow-y: ${value}`
}

alias: overflow
type: string
mapping: {
  undefined: `overflow: auto`
  [value]: `overflow: ${value}`
}
@kettanaito
Copy link
Owner

kettanaito commented Mar 12, 2019

Thanks for reaching out, @innerdaze.

I understand your concern, as I myself often face necessity to apply more styles than prop aliases provide. However, conceptually, prop aliases (and the library itself) is not meant to substitute all the CSS properties, and there is a strict listing of prop aliases on purpose, to prevent developers from stuffing compositions with tons of CSS properties.

There is still a solution to your problem. I recommend you to style your composition outside of the render. See the example below.

// Page.js
import styled from 'styled-components'
import { Composition } from 'atomic-layout'

const PageComposition = styled(Composition)`
  overflow-y: scroll;
`

const Page = () => (
  <PageComposition template={...}>{...}</PageComposition>
)

To be fair, I'm not blindly against utilizing the power of responsive props with any CSS properties. In order to retain clean philosophic vision of the library, and not sacrifice on practicality, I've suggested #126. This would allow to use custom prop aliases suitable for your project. That means that the library components would come with strict aliases by default, but you can extend them using the library's API.

@innerdaze
Copy link
Author

innerdaze commented Mar 12, 2019

I saw #126, that looks like a good plan.

For the solution you've suggested, I'm not sure that works for my example.

Here's what I ended up with:

import React, { useMemo } from "react";
import { Composition } from "atomic-layout";
import styled from "styled-components";

const templateMobile = `
  header
  menu
  content
  footer
`;

const gutter = 16;

const styleContent = comp => styled(comp)`
  overflow-x: ${({ scrollX, scroll }) =>
    scrollX || scroll ? "scroll" : "auto"}
  overflow-y: ${({ scrollY, scroll }) =>
    scrollY || scroll ? "scroll" : "auto"}
`;

const Page = () => (
  <Composition
    template={templateMobile}
    templateRows="auto auto 1fr auto"
    gutter={gutter}
    height="100vh"
  >
    {({ Header, Content, Menu, Footer }) => {
      const ContentStyle = styleContent(Content);

      return (
        <>
          <Header>
            {...}
          </Header>
          <Menu marginTop={gutter * -2}>
            {...}
          </Menu>
          <ContentStyle scrollY paddingHorizontal={30}>
            {...}
          </ContentStyle>
          <Footer>
            {...}
          </Footer>
        </>
      );
    }}
  </Composition>
);

export default Page;

Is there a way I could have applied the Content styling outside of the render?

@kettanaito
Copy link
Owner

kettanaito commented Mar 12, 2019

Yes, there is. Any generated area component supports polymorphic prop from styled-components. So you could do the following:

import styled from 'styled-components'
import { Composition } from 'atomic-layout'

// create a styled component outside the render
const StyledContent = styled.div`
  overflow-y: scroll;
`

const Page = () => (
  <Composition template={...}>
    {({ Content }) => (
      {/* Use polymorphic prop to render provided component */}
      <Content as={StyledContent} />
    )}
  </Composition>
)

Let me know that this works for your use case.

@innerdaze
Copy link
Author

ahh that's ideal, thanks - I hadn't realised this was possible

@kettanaito
Copy link
Owner

Glad that helped. I'm closing this issue in favor of #126. overflow CSS properties will be added as a part of pre-configured prop aliases packs once the referred API lands. Feel free to suggest more useful aliases in that issue.

Thank you once more for contributing!

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

No branches or pull requests

2 participants