Skip to content

Conversation

ethanshar
Copy link
Collaborator

Description

Fix docs links by adding missing category in "extendLink"

Changelog

Fix docs broken links

@ethanshar
Copy link
Collaborator Author

@Inbal-Tish
Notice that some link were still broken.
It usually tell you that when you try to deploy the docs (npm run docs:deploy) and you can test it by running the docs from docuilib folder

@Inbal-Tish
Copy link
Collaborator

@Inbal-Tish Notice that some link were still broken. It usually tell you that when you try to deploy the docs (npm run docs:deploy) and you can test it by running the docs from docuilib folder

I don't understand why add the category on these components to make the link work and not in all of the components?

@ethanshar
Copy link
Collaborator Author

@Inbal-Tish Notice that some link were still broken. It usually tell you that when you try to deploy the docs (npm run docs:deploy) and you can test it by running the docs from docuilib folder

I don't understand why add the category on these components to make the link work and not in all of the components?

What do you mean "all of the components"?
Where for example you'd expect it to be?

@Inbal-Tish
Copy link
Collaborator

Inbal-Tish commented Apr 26, 2022

@Inbal-Tish Notice that some link were still broken. It usually tell you that when you try to deploy the docs (npm run docs:deploy) and you can test it by running the docs from docuilib folder

I don't understand why add the category on these components to make the link work and not in all of the components?

What do you mean "all of the components"? Where for example you'd expect it to be?

I don't expect it to be used at all. What I'm asking is, take for example the Avatar component, we have the "extends" key with "ThouchableOpacity" so why do we need to specify the category, like ""basic/TouchableOpacity", in the Button component ?

@ethanshar
Copy link
Collaborator Author

I don't expect it to be used at all. What I'm asking is, take for example the Avatar component, we have the "extends" key with "ThouchableOpacity" so why do we need to specify the category, like ""basic/TouchableOpacity", in the Button component ?

Yes, you are right, there are some inconsistencies.. i'll check it out

@ethanshar
Copy link
Collaborator Author

ok few things

  • I pushed a fix for the buildDocs script with how it created the extends links, it didn't handle a case a components extends multiple components (like Avatar)
  • I fixed avatar api.json file (and badge), the extend links didn't even work

In general, when running npm run build from the docuilib folder it tells you which link are broken.
But apparently it has cache so it worth clearing it sometimes (npm run clear) otherwise you get wrong results


function generateExtendsLink(extendsLink) {
const extendedComponentName = _.last(_.split(extendsLink, '/')); // Incubator/TextField -> TextField
const extendsText = `[${extendedComponentName}](/docs/components/${extendsLink})`;
Copy link
Collaborator

@Inbal-Tish Inbal-Tish Apr 27, 2022

Choose a reason for hiding this comment

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

What about instead of getting the extendsLink as "basic/TextField" we will build it using its category. Something like:
const extendsText = "[${extendsText}](/docs/components/${component.category}/${extendsLink})";
In that case we won't have to add the category prefix to each component's extends ("extends": ["basic/View"]) and leave it as it is. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The component.category is the not necessarily the category of the component it extends.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right... Ok. I guess there's no other way than...

@Inbal-Tish Inbal-Tish merged commit dd03625 into master Apr 28, 2022
@ethanshar ethanshar deleted the fix/broken_doc_links branch April 2, 2024 06:38
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.

2 participants