executablebooks / sphinx-tabs

Tabbed views for Sphinx
https://sphinx-tabs.readthedocs.io
MIT License
264 stars 68 forks source link

Fix hiding tabs #106

Closed foster999 closed 3 years ago

foster999 commented 3 years ago

Changes:

Closes #105

Daltz333 commented 3 years ago

Should a test get verified that a tab gets hidden and unhidden properly?

foster999 commented 3 years ago

Yeah, we don't have any JS tests so this might be a good opportunity to get at least one in... Any experience with writing tests for JS? I'm yet to give it a try.

Also, looks like pygments has changed something so one of the regression tests is failing. Shall check if that affects appearance in any way.

codecov[bot] commented 3 years ago

Codecov Report

Merging #106 (3cefded) into master (d29f180) will increase coverage by 0.02%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
+ Coverage   97.10%   97.12%   +0.02%     
==========================================
  Files           1        2       +1     
  Lines         207      209       +2     
==========================================
+ Hits          201      203       +2     
  Misses          6        6              
Flag Coverage Δ
pytests 97.12% <100.00%> (+0.02%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sphinx_tabs/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d29f180...9369fa5. Read the comment docs.

foster999 commented 3 years ago

My attempt at testing the simplest function in the module is failing. Not sure if you have experience testing JS using jest @Daltz333?

I must be using this incorrectly, as the selectTab function has no effect on the button or panel:

const tabs = require('../tabs')

describe("Selecting a tab", () => {
    test("tab become selected", () => {
        // Set up our document body
        document.body.innerHTML =
        '<div>' +
        '  <button id="test-button" aria-controls="panel-id" aria-selected="false">Test</button>' +
        '  <div id="panel-id" hidden="true">Test</div>' +
        '</div>';

        let button = document.getElementById('test-button');
        let panel = document.getElementById('panel-id');
        expect(button.getAttribute('aria-selected')).toEqual('false')
        expect(panel.getAttribute('hidden')).toEqual("true")

        tabs.selectTab(button)

        expect(button.getAttribute('aria-selected')).toEqual('true')
        expect(panel.getAttribute('hidden')).toEqual("false")
    })
})
Daltz333 commented 3 years ago

I'm not familiar with Javascript beyond a basic extent. Mostly a python developer and documentation writer myself. If tests are blocking, we can always merge now and test properly later.

foster999 commented 3 years ago

Same here to be honest 😅 yeah that's fair, I'll ask a friend about the testing and get it in later.

Are you happy with the changes in this PR (fix has been verified by those who raised the issue) and #107?