Skip to content

Add browsable versions extension for scaladoc #12596

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

Merged
merged 3 commits into from
Jun 8, 2021

Conversation

BarkingBad
Copy link
Contributor

No description provided.

@BarkingBad BarkingBad force-pushed the scaladoc/drop-down-revision branch 2 times, most recently from 468fe96 to c46d686 Compare May 25, 2021 14:29
Comment on lines 60 to 63
document.onclick = (e: Event) => {
if e.target.asInstanceOf[html.Element].id != "dropdown-button" then
document.getElementById("dropdown-content").classList.remove("show")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Use document.addEventListener('click', (Event) => Unit) instead. onclick allows to attach only one handler per event so if anybody somewhere else did document.onclick, this handler would get overriden.

document is frequently used and it's very likely that somebody overrides it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change

@JSExportTopLevel("dropdownHandler")
def dropdownHandler() =
if document.getElementById("dropdown-content").getElementsByTagName("a").size > 0 &&
window.getSelection.toString.length == 0 then
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you need to check for selection?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To let people copy the version without clicking the button

Copy link
Contributor

Choose a reason for hiding this comment

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

that's a good point. It didn't occur to me.

Comment on lines 82 to 88
for i <- 0 until a.length do
val txtValue = a(i).innerText
val disp = if txtValue.toUpperCase.indexOf(filter) > -1 then
""
else
"none"
a(i).asInstanceOf[html.Anchor].style.display = disp
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that in Scala.js you can use HTMLCollection just like Seq, so you could just do a.foreach

scala-js/scala-js-dom#194
https://github.com/scala-js/scala-js-dom/blob/master/src/main/scala/org/scalajs/dom/ext/Extensions.scala

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

Comment on lines 82 to 88
for i <- 0 until a.length do
val txtValue = a(i).innerText
val disp = if txtValue.toUpperCase.indexOf(filter) > -1 then
""
else
"none"
a(i).asInstanceOf[html.Anchor].style.display = disp
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to use css class instead of inline style

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

@BarkingBad BarkingBad force-pushed the scaladoc/drop-down-revision branch from b396d86 to d165021 Compare May 27, 2021 12:30
@BarkingBad BarkingBad enabled auto-merge May 27, 2021 14:11
@pikinier20 pikinier20 self-requested a review June 8, 2021 08:34
Copy link
Contributor

@julienrf julienrf left a comment

Choose a reason for hiding this comment

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

Interesting! Do you mind adding more comments to the implementation class DropdownHandler, and also user documentation on how to use this feature? The most important thing is that we never document the expected format of the JSON document that provides the versions. An example would be welcome.

I’m curious to see how you eventually build the documentation for several versions. Did you experiment with some solutions already? It seems tempting to leave the content in the Git history, at specific tags, but then it means that to build the documentation you would have to perform multiple Git commands, and also sometimes you want to be able to keep improving the documentation even after a release. I think the other alternative would be to keep every version of the documentation in subdirectories? Then the process for creating a new version of the documentation would be to copy the current docs into a new subdirectory and update the JSON dictionary accordingly.

window.sessionStorage.setItem(KEY, UNDEFINED_VERSIONS)
disableButton()
}
catch // Globals.versionDictionaruUrl is undefined
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not check if versionsDictionaryUrl is set before calling getURLContent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The exception is not thrown by getUrlContent but by accessing versionsDictionaryUrl itself.
When I want to access the js proprty that actually does not exists (because not always we want to have more docs linked) we do not append js property to html header properties, thus when we want to access it we get and exception at js runtime. I don't know if I can handle this one better, I did it after inspecting the transpiled js code and it's the only solution I could come up with.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would a if Globals.versionDictionaryUrl == js.undefined work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will confirm that, but if I recall correctly calling Globals.versionDictionaryUrl throws the exception, not the comparison

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the type of versionDictionaryUrl to js.UndefOr[String] then did your comparision. As I said the exception is thrown by accessing the property at js runtime so I wrapped it with try catch
obraz

Copy link
Member

@sjrd sjrd Jun 8, 2021

Choose a reason for hiding this comment

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

In addition to changing it to js.UndefOr[String], use

if (js.typeOf(Globals.versionDictionaryUrl) != "undefined") {
  // it's defined
} else {
  // it's not
}

js.typeOf is magical, like JavaScript's typeof.

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, will try out

@BarkingBad
Copy link
Contributor Author

Hi @julienrf

I know I haven't provided yet any documentation, I was about to as soon as we implement it for dotty.epfl.ch docs to make sure that everything works smooth. For now I have mocked up things (will update with link after republishing snapshot). If you want to test it yourself you have to insert manually json to the session storage at key versions-json with value { "versions": { "3.0.0": "link", "3.0.1": "link2" }}

test

When it comes to the deploy, we provide dynamic interface for injecting dictionary of versions with url locations and it's up to user to manage the releases. User can keep them as he wish as long as they are available at their url path. For dotty, we will probably manage it with github actions by checking out git tree at provided list of tags, then getting tasty files, at last we will generate all the html docs using the newest scaladoc (for keeping the UI interface concise).

@BarkingBad BarkingBad force-pushed the scaladoc/drop-down-revision branch from d165021 to 84feae8 Compare June 8, 2021 12:08
@BarkingBad
Copy link
Contributor Author

@julienrf here is documentation deployed for tests if you are interested. Ofc as I said, you have to edit sessions storage by yourself since there is no config file provided yet

@BarkingBad BarkingBad disabled auto-merge June 8, 2021 13:00
@BarkingBad BarkingBad enabled auto-merge June 8, 2021 14:28
@BarkingBad BarkingBad merged commit 61a6833 into scala:master Jun 8, 2021
@BarkingBad BarkingBad deleted the scaladoc/drop-down-revision branch June 8, 2021 15:06
@Kordyjan Kordyjan added this to the 3.0.2 milestone Aug 2, 2023
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.

dotty.epfl.ch/docs should provide option to pick up a version that is documented
6 participants