react-breadcrumbs-dynamic icon indicating copy to clipboard operation
react-breadcrumbs-dynamic copied to clipboard

Pass props to Item

Open Wizyma opened this issue 6 years ago • 9 comments

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>

Wizyma avatar May 25 '18 13:05 Wizyma

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?

oklas avatar May 26 '18 13:05 oklas

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

Wizyma avatar May 26 '18 14:05 Wizyma

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.

oklas avatar May 26 '18 15:05 oklas

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

Wizyma avatar May 29 '18 07:05 Wizyma

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)

oklas avatar May 29 '18 08:05 oklas

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 !

Wizyma avatar May 29 '18 09:05 Wizyma

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

  • the feature increases the API/docs complexity
  • such kind of wrapper can be added inside a specific project with a few lines of code
  • the behavior is not totally predictable for all of the consumers

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

zmeecer avatar Feb 24 '19 10:02 zmeecer

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.

oklas avatar Feb 24 '19 17:02 oklas

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... !

Wizyma avatar Feb 25 '19 08:02 Wizyma