Skip to content

Enable Style Customization#659

Open
SharpSeeEr wants to merge 3 commits intonamespace-ee:mainfrom
SharpSeeEr:develop
Open

Enable Style Customization#659
SharpSeeEr wants to merge 3 commits intonamespace-ee:mainfrom
SharpSeeEr:develop

Conversation

@SharpSeeEr
Copy link

Issue Number

#415

Overview of PR

I just recently discovered this library, and it is the perfect fit for my project. I did find it strange that the only way to custom style the component was using an itemRenderer, groupRenderer, and passing specific styles. Since it is a component library meant to be used in any project styling should be the simplest thing to override and customize, IMO.

So I have moved all styles that can be safely overridden from jsx to scss. Overriding the default styling is now as simple as overriding the css classes.

I also removed the use of composeEvents() in items.js, as it caused creation of new functions to pass as props to itemRenderer on every render. I know, I know, multiple changes in one PR! Horrible. But it seemed to give a decent performance increase. I have absolutely zero hard facts to back that claim up.

I'm using the timeline with several thousand groups with anywhere from 3 to 20 items per group, so performance is very important to me, and performance is a little slow right now.

Move styling out of components to scss
 - This will allow styling overrides to be specified using classes and
   css.

Using composeEvents() was creating a new function reference being passed
to the itemRenderer.
@Ilaiwi
Copy link
Collaborator

Ilaiwi commented Sep 27, 2019

@SharpSeeEr I am glad you found what you need from the library. Thank you for this. I will look into the PR and review it but the compose event is there for a reason... It is needed for you to be able to pass event handlers to the item that we already use like mouseup, mousedown... you can provide memoize function for the compose and it will solve your problem.
For the styles I haven't taken a good look yet but thank you for finding the issue related...

@SharpSeeEr
Copy link
Author

I did see the purpose of composeEvent(), and I updated the sections of code that called it to still check for and call the passed in event handlers for mouseup, mousedown, etc.

I am a veteran developer, but a junior react developer - feedback is always welcome.

I also read the contributing guidelines today (one day too late) and see that pull requests should go to master. I would be more than happy to close this PR and reopen a new one to master if needed.

@Ilaiwi
Copy link
Collaborator

Ilaiwi commented Sep 27, 2019

@SharpSeeEr develop branch is okay no worries.
also no need to run the linting rules checker.
So event handlers can come in 2 ways:

  • through item.props
  • propsGetter

You handled the first case but not the second. I would suggest just memorizing the function composeEvent for each handler. I am fine with adding memoize-one as a dependency...
This is still not an in-depth code review I will get to that soon I promise 😇

@SharpSeeEr
Copy link
Author

Thanks! I'll look into the memoization and update the pull request... soonish. I appreciate this early feedback just as much!

@berdyshev
Copy link

any updates on that? I think it's nice to have

@lnagel lnagel added the outdated label Feb 5, 2026
@lnagel
Copy link
Member

lnagel commented Feb 5, 2026

This PR has been marked as outdated. Here's why:

The codebase has been significantly rewritten for v0.30.0-beta.5, and the changes in this PR are no longer applicable:

  • All source files this PR modifies (src/lib/Timeline.js, src/lib/items/Item.js, src/lib/columns/Columns.js, src/lib/headers/*.js, src/lib/scroll/ScrollElement.js, etc.) have been rewritten from JavaScript to TypeScript (.tsx/.ts)
  • The Timeline.scss file has already been reworked in the current codebase — it now uses SCSS variables with !default flags (e.g., $item-color, $item-background, $border-color, $date-header-background, etc.), which is essentially the same goal this PR had of enabling style customization through overridable variables
  • The composeEvents() utility that this PR removed still exists in the current codebase (src/lib/utility/events.ts) and is used in Item.tsx and Interval.tsx, but the implementation is now typed with TypeScript generics
  • The build system has moved from Webpack to Vite, and the SCSS is now properly exported in the package (dist/Timeline.scss, dist/style.css)

The style customization goal of this PR has been largely achieved through the current SCSS variable approach with !default flags. The diff against .js files cannot be applied to the current .tsx source.

Thank you for the contribution! The idea/fix was appreciated even if the codebase has moved on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants