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: version switcher #11

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from

Conversation

mischah
Copy link
Contributor

@mischah mischah commented Nov 18, 2016

Hej, here’s the implementation of option 3 of the version switcher as proposed in #5.

Must say that I prefer it over the implementation of option 1 in #9

Pros:

  • You can switch from older version to newer
  • Less changes in publish.js
    • Makes it easier to integrate future changes from JSDocs default template

Cons:

  • We are loosing the custom page titles from the docs
    • Cause we only see the output from the <title> Element of the page holding that iFrame
  • We are loosing bookmarkability and deeplinking to subpages

Closes #5

@mischah
Copy link
Contributor Author

mischah commented Nov 18, 2016

Ready to review :octocat:

@mischah mischah force-pushed the version-switcher-option-3 branch from 73282b5 to 351443b Compare November 18, 2016 01:28
@mischah
Copy link
Contributor Author

mischah commented Nov 18, 2016

v1 0 4

and improve a few other things.
@mischah mischah force-pushed the version-switcher-option-3 branch from ec58c6a to 81df2a2 Compare November 18, 2016 02:38
1. Use `npm run serve` or `gulp serve` command to ascertain realtime.
3. Api-Example tab, Auto-Complete and Resize functions are written in the `static/scripts/tui-doc.js` file.
1. Use `npm run serve` or `gulp serve` command to wacth for changes and live reload browser.
3. API-Example tab, Auto-Complete and Resize functions are written in the `static/scripts/tui-doc.js` file.
Copy link

@minkyu-yi minkyu-yi Nov 21, 2016

Choose a reason for hiding this comment

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

I greatly appreciate for your correction 🤗 .
(I'm not good at english 😅..)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😊

Copy link

Choose a reason for hiding this comment

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

There's still a typo in watch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks 😘
… going to work on that PR tonight.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was outdated ;)

@minkyu-yi
Copy link

minkyu-yi commented Nov 21, 2016

option3 vs option1

Must say that I prefer it over the implementation of option 1 in #9

Yeah, I prefer the option1 too. I was just busy for a while and could not see the #9.
However, this option3 is good too as the option1.
So I think the option3 should be pended for a while and reopen the #9.
(Sorry too late,, I should have seen the #9 early)

-->
(update) I misunderstood the above sentence 😖.
Ok. This option3 will be a good feature. Thanks~


Another proposal

Readme.md - line 67

The version switcher gets the versions via Git tags. But JSDoc only generates the current sources. So keep in mind that you have to generate the docs for each tag. Otherwise switching versions will lead to 404 errors.

How about the ignore option in versionSwitcher?
Bc the patch versions or build versions don't need to generate new docs.

If agree, at least how about adding a [string|array] values to ignore option?

In configuration,

--- no ignore.

"versionSwitcher": "true"

--- ignore some versions.

"versionSwitcher": {
    "ignore": "minor"
}

or

"versionSwitcher": {
    "ignore": "patch"
}

or

"versionSwitcher": {
    "ignore": ["1.0.1", "1.0.1+12312", "..."]
}

If you are busy, I can take over this based on #9 🙂.

@minkyu-yi minkyu-yi added this to the 1.1.0 milestone Nov 21, 2016
@minkyu-yi minkyu-yi removed this from the 1.1.0 milestone Nov 21, 2016
@mischah
Copy link
Contributor Author

mischah commented Nov 21, 2016

I think this approach is better that #9
See pro arguments.

Do you agree? In this case I would implement the ignore option 😊

@mischah
Copy link
Contributor Author

mischah commented Nov 21, 2016

I really like the idea of the ignore option.

Plus having the possibility to ignore patch versions or specific versions.

But it would be more useful to accepts semver syntax and ignore version levels like this

"versionSwitcher": {
    "versions": [">=2.x"]
}

This would result in having all versions up from 2.0.0. See npm semver calculator

Or

"versionSwitcher": {
    "versions": [">=2.x"],
    "excludeLevel": "patch"
}

This would result in having all versions up from 2.0.0 excluding patch level releases.

@minkyu-yi minkyu-yi removed the pending label Nov 22, 2016
@minkyu-yi minkyu-yi added this to the 1.1.0 milestone Nov 22, 2016
@minkyu-yi
Copy link

minkyu-yi commented Nov 22, 2016

Awesome~
I agree this semver-syntax + excludeLevel option. 👍

@mischah
Copy link
Contributor Author

mischah commented Nov 27, 2016

Guess I’m done with that :octocat:

Anyone wants to review? 😗

@mischah mischah force-pushed the version-switcher-option-3 branch 2 times, most recently from 3a6dd99 to 4126577 Compare November 27, 2016 00:51
@mischah mischah force-pushed the version-switcher-option-3 branch from 4126577 to 6bf30fd Compare November 27, 2016 00:54
@mischah mischah force-pushed the version-switcher-option-3 branch from ad62543 to 4c7bb09 Compare November 27, 2016 18:32
@mischah
Copy link
Contributor Author

mischah commented Nov 29, 2016

Hej @minkyu-yi,

any idea when to land this approximately?

Sorry, I don’t wanna put any pressure on you but I’m so excited to use that template for the Yeoman docs 😗

@minkyu-yi
Copy link

@mischah

Yeah, I hope to release no later than next week. Sorry for the delay.

@mischah
Copy link
Contributor Author

mischah commented Dec 2, 2016

👌 Thanks for the info.

@minkyu-yi
Copy link

Hej @mischah

Thanks for the pr.
And I found a problem about url. (may be relative to the your concern.)

We could access a specific method, class, etc from url without the iframe-version-switcher.
image

But we can not access from url with the iframe-version-switcher.
image

This problem arises from iframe.
Sorry, I didn't realize this problem in advance.

So I propose the version-switcher form(<form class="form-inline version-switcher">) in layout.tmpl instead of iframe.

This form rendered in layout.tmpl like below if the version-switcher option is truthy.

<body>
  <?js if(versionSwitcher) { ?>
      <header>
          <form class="form-inline version-switcher">
              <div class="form-group">
                  <label for="selectVersion">Version:</label>
                  <select id="selectVersion" class="form-control"></select>
              </div>
          </form>
      </header>
  <?js } ?>

  ...left-navigation,
  ...main
  ...footer
</body>

Or any ideas to solve this problem?
I'm looking forward to your opinion.

@mischah
Copy link
Contributor Author

mischah commented Dec 5, 2016

This problem arises from iframe.
Sorry, I didn't realize this problem in advance.

No problem. Me neither. Just updated the initial PR description.

So. Yes. Native browser deeplinking and bookmarkability is another thing we lose with the iFrame approach.
But it has other pros. See first PR description. ⬆️

Must say that I don’t want to trash the iFrame approach because of its pros.

Possible solution

With f46954c I made it possible to link to older versions.

I could try to extend this functionality to:

  • Adress bookmarkability:
    • Update the iFrame URL (visible in the adress bar) with every click inside the iFrame
  • Adress deeplinking:
    • Pass URL options from iFrame URL (visible in the adress bar) to the iframe src on page load

Is this something you would accept as solution?

@minkyu-yi
Copy link

Yeah~!😀

In this problem, with your solutions, using the iframe is not a problem. We can sync parent url and iframe url with using your solutions.

So we can share a specific url with others for seeing the same page within version-switcher.

Is it right? I think it would be a really useful feature.

I appreciate your help.

@mischah
Copy link
Contributor Author

mischah commented Dec 5, 2016

So we can share a specific url with others for seeing the same page within version-switcher.

Yeah. Thats how it should work. Can’t promise anything for now, though. But I would like to give it a try.

Back to the example you posted.

URL without version switcher:

http://localhost:8080/tui.component.BaseChild.html#getDatum

URL with version switcher enabled:

http://localhost:8080?v=1.3.0&page=tui.component.BaseChild.html#getDatum

or similar


Im not quite sure how to implement that. I possibly could use the HTML5 history API.
But this is only supported by IE10 and above. Do you have any problem if this won’t work with IE9 and below?

@minkyu-yi
Copy link

minkyu-yi commented Dec 7, 2016

I think most use cases of jsdoc-template are on modern browsers. So the version-switcher doesn't need to support old browsers.🙂

@minkyu-yi
Copy link

minkyu-yi commented Jan 13, 2017

@mischah
Hi.
long time no see.
Soon it is time to minor update for other features.
So, I just wonder when we get this feature? I'm looking forward your comment.

Thank you :)

@minkyu-yi minkyu-yi modified the milestone: 1.1.0 Jan 13, 2017
@mischah
Copy link
Contributor Author

mischah commented Jan 13, 2017

Thanks for pinging me.
I’m on parental leave for the next 3 weeks and quite busy with parenting 😄

I picked up a newborn and the mother from hospital the day before yesterday plus I need to take care of my 2 year old son. Hope that things are getting more relaxed next week. But I can't promise anything by now. Sorry 🙈

@minkyu-yi
Copy link

Oh, I see.
Congratulations on your newborn 🎉
I hope you get more relaxed, too.

Thank you!

@mischah
Copy link
Contributor Author

mischah commented Nov 9, 2017

Hej, it’s been almost a year since I started working on this 🙈

Just wanna say thank you for keeping this open.
I guess finally find time to finish this next month 😘

@alex-taxiera
Copy link

@mischah bump ;)

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.

4 participants