Feature/subnav component - #42
Conversation
-- Added new web pack config for navbar components, to publish it as library -- Fixed issues with NavBar and SearchBar
-- Trying with travis.yml
-- Parse error fixes
-- Fixed module name
-- Changed bucket name
Feature/bundle navbar component
-- Carousel component created -- It behaves same on both mobile and desktop -- Swipe is not yet supported for mobile devices
-- Fixing lint errors
-- Added component for SubNav using the Carousel component.
-- Fixed lint errors
-- Fixed padding
| } | ||
|
|
||
| validatePagers() { | ||
| const pageDownClass = classNames({ |
There was a problem hiding this comment.
You can comma separate the arguments, so for something like a class that always needs to be added you can do:
classNames(
'page-down',
{ hidden: this.state.firstVisibleItem === 0 }
)
|
I added a few comments. Overall it lgtm! |
-- Implemented code review suggestions
|
Thanks @nlitwin for the review. |
|
Hmm regarding the multiline texts, I'm not sure unfortunately :/ I'm going to have to include a multiline ellipsis React module soon, so maybe I'll glean some insights from that! |
|
Closing PR without merging because it is created against master which is causing multiple change sets to be included in one PR. This branch was created out of dev, so PR would be created against dev. |
2 participants
@nlitwin @aselbie
-- Carousel component created
-- It behaves same on both mobile and desktop
-- Swipe is not yet supported for mobile devices
-- Added component for SubNav using the Carousel component.
Please let me know if I can improve the way the Carousel is implemented, I mean can it be made stateless or something else that is the best practice for React.
Further, there is issue with dynamic height elements in the carousel e.g. when text of items can be adjusted to multiple lines, there is flicker in the rendering of prev/next buttons. It works fine for single lined text. I would try to resolve it tomorrow.