-
Notifications
You must be signed in to change notification settings - Fork 188
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
got rid of bootstrap.min.js #500
base: next
Are you sure you want to change the base?
Conversation
Obviously, the custom.css file text will need to be merged into elegant.css. I just didn't want to screw up upstream pull requests.
@andrewdstrain couple of feedbacks
Feature parity
I suggest taking advantage of unused CSS tools to extract the relevant CSS from bootstrap. Also, have a look at this snippet. It is much simpler and cleaner |
comments are done javascript should still work but needs testing
By me doing CSS only, the JavaScript would need to make sure that the checkbox is checked when already open. Then it would close when already open. If it's simply shown but not checked, the first click will check the checkbox, which opens everything. In custom.css, I did get rid of all the tag selectors. It's only using bootstrap.min.css selectors now. I did remove the disqus.comment.count class because the counts never work - at least for me - even without all of these changes (on master). That class is used by the JavaScript from Disqus - not you. So it's not really your problem. But getting rid of it does fix it. I've never done a pull request before. But I installed the hooks and am trying to follow the guidelines. If I'm not doing that right, please let me know what I'm doing wrong. I agree - the other snippet is simpler and cleaner. I'm going to look into that. I really appreciate this. Thank you so much! |
JS is not needed. As you can see in the snipper I shared.
No issues. Take your time. In case anything is not clear, feel free to post here to contact me on my email |
I looked at the code at CodePen and it's doing pretty much the same thing as I'm doing. Once you remove all the display related CSS, it looks like this: https://www.w3schools.com/code/tryit.asp?filename=G7C87MRRMZ4R. See the grocery list? It doesn't display as it should. That's because only HTML elements are used in the CSS. Thus, you need to use classes. Once you use classes, you don't need the elements in the CSS and can change the elements in the HTML if needed. So look at this: https://www.w3schools.com/code/tryit.asp?filename=G7C8JCTKRP8G. The Grocery list is displayed as normal and the HTML elements are different but everything still works. The two URL links (above) are identical except that elements are used like in the website you showed me and classes are used in the one I changed. Everything else is the same. And don't think that I cheated with the grocery list by changing the elements in the HTML. Look at this: https://www.w3schools.com/code/tryit.asp?filename=G7C8N8WUZUN4. You can clearly see that the HTML elements match now and the grocery list displays as it should. This is because the CSS is based on classes and not elements. There are some minor differences in what the one you showed me does and what I posted to you. One difference is not needing the I got rid of the I also got rid of the cool animations - that's only to keep the examples simple because what we're talking about is how the accordion works, not how cool it looks. All that stuff can be added back in. |
Having said all of that, I am going to change the CSS to follow the CodePen example. I highly agree that it’s easier to read and follow. |
This is very important. I am glad we both are on the same page here. Bug
Expected result: It would close. Next stepMatch the CSS with bootstrap. Here is how to do it.
Now, the new categories should match the old ones in look and feel. It will also take care of the animation, hopefully. <html>
<body>
<div class="accordion" id="accordion2">
<div class="accordion-group">
<div class="accordion-heading">
<a class="accordion-toggle" data-toggle="collapse" data-parent="#accordion2" href="#collapseOne">
Collapsible Group Item #1
</a>
</div>
<div id="collapseOne" class="accordion-body collapse in">
<div class="accordion-inner">
Anim pariatur cliche...
</div>
</div>
</div>
<div class="accordion-group">
<div class="accordion-heading">
<a class="accordion-toggle" data-toggle="collapse" data-parent="#accordion2" href="#collapseTwo">
Collapsible Group Item #2
</a>
</div>
<div id="collapseTwo" class="accordion-body collapse">
<div class="accordion-inner">
Anim pariatur cliche...
</div>
</div>
</div>
</div>
</body>
</html>
|
That's a feature, not a bug! (joke) It's because the radio button doesn't uncheck like checkboxes do. There's another way to do the CSS. It doesn't use input type radio but anchors instead. It suffers from the same bug because once the anchor is clicked on, clicking on it again just selects the same anchor. |
Right. Then let's use checkboxes (like here https://codepen.io/abergin/pen/ihlDf) and use CSS classes. |
change input type checkbox to hide on checked default checkboxes to checked change navbar to show new checkbox logic leave input type radio to use not checked to hide
categories.html: collapse() is not a function
@andrewdstrain is the PR ready now? |
As far as I can tell, yes. I'm also getting rid of Bootstrap CSS. I'm now merging all the custom.css changes into the elegant.css file. Right now, I'm on the step of getting rid of CSS that is no longer needed in the elegant.css file. The next step is to merge the changes in custom.css into elegant.css. You can check out the changes using the You (or someone else) might be able to help me with this as my time is a bit less than it used to be. |
@andrewdstrain I just tried the deploy preview. https://deploy-preview-500--pelicanelegant.netlify.com/categories The categories are not working. Clicking on each do change the URL but does not expand the accordion. Let me know if you can look into it, otherwise, I will fix the issue on top of your PR. |
@andrewdstrain any update on this? If you can fix the issue I stated above, then I will fix the CSS style issue myself. |
What is the status on this Request, is there anything I can help with? |
|
Prerequisites
next
branchRecommended Steps
Description