Skip to content

Conversation

@mbensch
Copy link
Contributor

@mbensch mbensch commented Aug 10, 2021

WHY are these changes introduced?

Fixes #4244

Passing the selected prop to Navigation.Item had no effect if the url prop was omitted.

WHAT is this pull request doing?

Adds Item-selected classname for items without url when selected prop is true.

@ghost
Copy link

ghost commented Aug 10, 2021

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines.

@mbensch mbensch changed the title [WIP] [Navigation.Item] Support selected prop for navigation items without url [Navigation.Item] Support selected prop for navigation items without url Aug 10, 2021
@mbensch mbensch force-pushed the fix-nav-item-selected-prop branch from 4e1ca82 to 140f125 Compare August 10, 2021 20:57
@mbensch mbensch marked this pull request as ready for review August 10, 2021 20:58
@mbensch mbensch requested a review from AndrewMusgrave August 10, 2021 20:58
Copy link
Member

@AndrewMusgrave AndrewMusgrave left a comment

Choose a reason for hiding this comment

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

Awesome work 🎉

@AndrewMusgrave
Copy link
Member

It looks like the check may have stalled. If you push up your branch again they should re-run

@mbensch mbensch force-pushed the fix-nav-item-selected-prop branch from 140f125 to 40ff793 Compare August 11, 2021 16:41
@mbensch
Copy link
Contributor Author

mbensch commented Aug 11, 2021

Rebased and pushed. Hope all checks run this time.

@github-actions
Copy link
Contributor

size-limit report

Path Size
cjs 142.54 KB (+0.01% 🔺)
esm 96.31 KB (+0.01% 🔺)
esnext 139.51 KB (+0.01% 🔺)
css 33.75 KB (0%)

@AndrewMusgrave
Copy link
Member

Github had degraded services yesterday, but it looks like everything is running now 🙌

@mbensch mbensch merged commit c97de31 into main Aug 11, 2021
@mbensch mbensch deleted the fix-nav-item-selected-prop branch August 11, 2021 17:02
@ghost
Copy link

ghost commented Aug 11, 2021

🎉 Thanks for your contribution to Polaris React!

lucabezerra pushed a commit that referenced this pull request Aug 16, 2021
- Support selected prop for navigation items without url
- Update UNRELEASED.md
BPScott pushed a commit that referenced this pull request Aug 17, 2021
- Support selected prop for navigation items without url
- Update UNRELEASED.md
CSilivestru pushed a commit that referenced this pull request Aug 25, 2021
- Support selected prop for navigation items without url
- Update UNRELEASED.md
CSilivestru pushed a commit that referenced this pull request Aug 26, 2021
* Support selected prop for navigation items without url (#4375)

- Support selected prop for navigation items without url
- Update UNRELEASED.md

* Allow subnav items to execute provided onClick handlers

* Update UNRELEASED.md

* Update tests to reflect number of called times

* Removed outdated bug fix that is now in main

Co-authored-by: Marcel Bensch <mbensch@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants