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

[docs] Add an index page which contains links to all examples #125 #126

Merged
merged 4 commits into from
Jun 15, 2022

Conversation

totallynotvaishnav
Copy link
Member

closes #125

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Can you please update the README to mention these examples and provide instructions to anyone who may want to see these?

I am talking about: https://github.com/openwisp/netjsongraph.js/tree/gsoc22#install
I think we should rename that section something like "Install and run demo examples"
Can you also make the start command open the browser to the examples page as we have in wifi-login-pages (which opens the default organization)?

@totallynotvaishnav
Copy link
Member Author

am talking about: https://github.com/openwisp/netjsongraph.js/tree/gsoc22#install
I think we should rename that section something like "Install and run demo examples"

Should I update only that specific part?

Can you also make the start command open the browser to the examples page as we have in wifi-login-pages (which opens the default organization)?

We already have this.

@nemesifier
Copy link
Member

am talking about: https://github.com/openwisp/netjsongraph.js/tree/gsoc22#install
I think we should rename that section something like "Install and run demo examples"

Should I update only that specific part?

Yes for now. Do you think there's any other relevant part that needs to be updated?

Can you also make the start command open the browser to the examples page as we have in wifi-login-pages (which opens the default organization)?

We already have this.

Ok!

@totallynotvaishnav
Copy link
Member Author

Yes for now. Do you think there's any other relevant part that needs to be updated?

I don't see any other parts now.

@totallynotvaishnav totallynotvaishnav added this to In progress in [GSoC 22] netjsongraph.js via automation Jun 10, 2022
@nemesifier nemesifier changed the title Add an index page which contains links to all examples #125 [docs] Add an index page which contains links to all examples #125 Jun 13, 2022
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Please rebase on the latest master and fix conflicts.
You can prefix this PR and commit message with [docs] .

[GSoC 22] netjsongraph.js automation moved this from In progress to Review in progress Jun 13, 2022
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Please let's give some love to this thing:

Screenshot from 2022-06-14 19-05-22

If you were a user of this library, looking at this thing would you be excited or put down?

Can you make it a bit more readable and human friendly please?

...JSONData,
nodes,
links
}));
Copy link
Member

Choose a reason for hiding this comment

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

--skip-checkendline is disabled and hence not complaining about this but we should maintain consistency across openwisp modules and ensure files always have an ending new line.

Copy link
Member Author

Choose a reason for hiding this comment

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

I copied this from openwisp-wifi-login-pages. It is there to avoid checking end-lines for files in node modules. We already have insert_final_newline = true in .editorconfig to insert new line.

@totallynotvaishnav
Copy link
Member Author

Screenshot
Screenshot 2022-06-15 at 9 07 46 PM

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

@totallynotvaishnav let's focus a bit on the text now, why is "LInk to demo ..." repeated each times? We know those are links to demos.

Also, I think the font of the links would be better a sans-serif font. Regarding the title "Basic examples" doesn't souond right to me, I think we should call it "Netjsongraph.js Example Demos" or something similar.

</header>
<main>
<div class="cards">
<a href="./netjsongraph.html" target="_blank">Link to netjsongraph base demo</a>
Copy link
Member

Choose a reason for hiding this comment

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

this could be just "Basic usage"

<a href="./netjsongraph.html" target="_blank">Link to netjsongraph base demo</a>
</div>
<div class="cards">
<a href="./netjsonmap.html" target="_blank">Link to netjsonmap base demo</a>
Copy link
Member

Choose a reason for hiding this comment

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

Is this the geo map? If yes, we could name it just "Geographic map"

Copy link
Member

Choose a reason for hiding this comment

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

And so on for the other links.

@totallynotvaishnav
Copy link
Member Author

Updated screenshot
Screenshot 2022-06-16 at 2 04 11 AM

[GSoC 22] netjsongraph.js automation moved this from Review in progress to Reviewer approved Jun 15, 2022
@nemesifier nemesifier merged commit 4c92812 into gsoc22 Jun 15, 2022
[GSoC 22] netjsongraph.js automation moved this from Reviewer approved to Done Jun 15, 2022
@nemesifier nemesifier deleted the issues/125-add-index-page branch June 15, 2022 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants