Skip to content
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

feat(sidebar): Toggle button #1268

Merged
merged 7 commits into from
Jun 13, 2019
Merged

feat(sidebar): Toggle button #1268

merged 7 commits into from
Jun 13, 2019

Conversation

ConradJChan
Copy link
Contributor

@ConradJChan ConradJChan commented Jun 12, 2019

TODO:

  • unit tests
  • e2e test

Sidebar toggle

@ConradJChan ConradJChan requested a review from a team as a code owner June 12, 2019 20:30
@ConradJChan ConradJChan requested a review from a team June 12, 2019 20:30
@boxcla
Copy link

boxcla commented Jun 12, 2019

Verified that @ConradJChan has signed the CLA. Thanks for the pull request!

@booc0mtaco
Copy link
Contributor

Will this preserve the behavior where clicking on a highlighted section (like the activity feed chat bubble), will also hide/show the sidebar?

@ConradJChan
Copy link
Contributor Author

@booc0mtaco yep!

jstoffan
jstoffan previously approved these changes Jun 13, 2019
jstoffan
jstoffan previously approved these changes Jun 13, 2019
<div className="bcs-SidebarNav-tabs">
{hasActivity && (
<SidebarNavButton
data-resin-target={SIDEBAR_NAV_TARGETS.ACTIVITY}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decoupled the data-testid's from the constants used for the data-resin-target's

const SidebarNavButton = ({ children, interactionTarget, isOpen, onNavigate, sidebarView, tooltip }: Props) => {
const SidebarNavButton = ({
children,
'data-resin-target': dataResinTarget,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can probably use ...rest, instead? Up to you.

@mergify mergify bot merged commit e2ad4ab into box:master Jun 13, 2019
j3tan pushed a commit to j3tan/box-ui-elements that referenced this pull request Jul 9, 2019
* feat(sidebar): Toggle button

* chore: PR comments and add unit tests

* chore: fix classes

* chore: add e2e tests

* chore: update README.md

* chore: decouple resin tags from data testids
alexkrolick pushed a commit to alexkrolick/box-ui-elements that referenced this pull request Jul 10, 2019
* feat(sidebar): Toggle button

* chore: PR comments and add unit tests

* chore: fix classes

* chore: add e2e tests

* chore: update README.md

* chore: decouple resin tags from data testids
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants