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

Bootstrap4 Navigation bar added (adapted from Bootstrap3 Navigation bar) #120

Open
wants to merge 2 commits into
base: release-candidate
Choose a base branch
from
Open

Bootstrap4 Navigation bar added (adapted from Bootstrap3 Navigation bar) #120

wants to merge 2 commits into from

Conversation

sanjayminni
Copy link

Hi I have adapted a Bootstrap4 Navigation Bar from the Bootstrap3 Navigation Bar - you can check if its useful and adopt it into the willow-bootstrap. thanks

@fortizpenaloza
Copy link
Member

Hi @sanjayminni , thanks for contributing.

Can you specify what do you mean by "adapted"? Bootstrap 4 describes a navbar here https://getbootstrap.com/docs/4.0/components/navbar/ . Does your solution conforms with it?

Last but not least, could you write some unit tests?

@sanjayminni
Copy link
Author

sanjayminni commented Jul 23, 2021 via email

@mtabacman
Copy link
Member

Hi Sanjay.

You'd need to create a Bootstrap4NavigationBarTest class in the Willow-Bootstrap-4-Tests package, based on the tests included in Bootstrap3NavigationBarTest.

The expected html code would be the one presented in the Bootstrap site for that feature, trying to use as little components as possible, so that we are testing mainly the way the navigator bar is rendered, but keeping it "valid" (that is, no empty bar unless that makes sense in a bootstrap 4 app, etc.).

Lets us know if that answers your question, and many thanks for your participation in the project!

@sanjayminni
Copy link
Author

Hi Ignore this PR. I had only made a couple of changes in 1 class but this is showing the whole lot. something wrong in the way I forked / committed etc.

this https://discord.com/channels/223421264751099906/311419918631305218/879654296566784020
and subsequent messages may have something to do with this

anyway will try to redo and submit with test cases. am deleting this clone

@gcotelli gcotelli removed their request for review December 23, 2021 12:01
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.

3 participants