oklas / react-breadcrumbs-dynamic

:house_with_garden: > breadcrumbs > extremely flexible > and > easy to use
MIT License
126 stars 30 forks source link

Pass props to Item #22

Closed Wizyma closed 3 months ago

Wizyma commented 6 years ago

Hi,

First of all really nice what you've done. i have a suggestion for a feature / improvement. We could add ...props to item to allow us to pass custom props (style more precisely for my case !!) to it.

use cases I have a NavLink if i want to style it to not have any decoration in the text i have to do :

const Nav = props => (
  <NavLink
    {...props}
    style={{ textDecoration: 'none' }} >
    {props.children}
  </NavLink>
);

<Breadcrumbs separator={<strong> / </strong>} item={Nav} finalItem="strong" />

How to solve it Add for example itemProps to Breadcrumbs

const itemProps = props.itemProps 

    <Container {...containerProps}>
      {itemsValue.map((itemValue, i) => {
        return i+1 < count ? (
          separator ? (
            <span key={i}>
              <Item {...prepareProps(itemValue, rename, duplicate, remove)} {...itemProps} />
              {separator}
            </span>
          ) : (
            <Item key={i} {...prepareProps(itemValue, rename, duplicate, remove)} {...itemProps} />
          )
        ) : (
          <FinalItem key={i}
            {...prepareProps(itemValue, rename, duplicate, remove)}
            {...finalProps}
          />
        )
      })}
    </Container>
oklas commented 6 years ago

Hi @Wizyma, have a nice day. So you suggests to add itemProps to Breadcrumbs component to make it possible to define default props for item. It is good suggestion. Only one thing concern your snippet. The default item props must be applied at the begin, i.e. before props which brings from item agent (before props returned by prepareProps).

Let me know, whould you like to submit pr with tests for this feature?

Wizyma commented 6 years ago

hi @oklas

The default item props must be applied at the begin, i.e. before props which brings from item agent (before props returned by prepareProps).

You're right i didnt noticed it ! From what you say it means that the item should be for example styled before it is call with prepareProps ?

i'll try to look into it when i'll have some free time

oklas commented 6 years ago

The item will be styled by default if <Breadcrumbs itemProps={{style:{...}}}> is specified. And it will be possible to replace style from <BreadcrumbsItem style={{....}}>.

Make sure that you have actual commit from top of master. Take a look at previous pr, it may be helpfull.

Wizyma commented 6 years ago

@oklas can we use by default EmotionJs for the styling ? Or it will be an inline style injected directly into the jsx tag ?

oklas commented 6 years ago

It does not concern breadcrumbs implementation. We only combine and pass props. So users can choose what they wants, inline style or any css class or any thing.

Render the breadcrumbs itself with default props for items:

const className = css({color: 'green'})
const App = props => (
  <Breadcrumbs itemProps={{className}}>
)

Reneder some page with some breadcrumbs item with specific prop (className):

const className = css({color: 'red'})
const Page = props => (
  <BreadcrumbsItem {...{
    className
  }}>
)

(This uses shortland with spread)

Wizyma commented 6 years ago

Alright i've got everything i need to know :smile: , I have a lot of work going on, when i'll have more free time i'll dot it !

zmeecer commented 5 years ago

Actually, I don't think that is should be a responsibility of the library because of:

But is @oklas (as an awesome maintainer, frankly saying) agrees with the features, I can make a PR during a week. Otherwise, let's close the issue like a not really popular feature request

oklas commented 5 years ago

Hi All, I think if @Wizyma will not confirm ticket is in work for a few days it is free for grab. @zmeecer are you about styling and css I agree requirements is not clear and seems it is not concern library. And about item itemProps - it is just default props for item, exactly like finalProps which was not added at the beginning. As you noticed, the only things about above snippet is the confusion of sequence of props appling. Default props (itemProps) must be applied before comers.

Wizyma commented 5 years ago

Hi @oklas, i am so sorry i completly forgot about it, was on other projects for work... ! I managed to solve it back then just by simply styling my items before i pass it to the breadcrumb... (sounds so dumb >< !)! I think the issue is not relevant anymore... !