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

Riju frontend enhancement #91

Closed
inaseem opened this issue Jul 27, 2021 · 27 comments
Closed

Riju frontend enhancement #91

inaseem opened this issue Jul 27, 2021 · 27 comments

Comments

@inaseem
Copy link

inaseem commented Jul 27, 2021

Hi, @raxod502 congrats on bringing Riju live again. I have spent a day recreating the Riju front end.
Here is the demo URL.
https://inaseem.gitlab.io/riju

Improvements:

  1. Redesign the Home page
  2. Redesign the Editor and Terminal page.
  3. Added Search as scrolling to find a language option to click is a little tedious.
  4. Added colors to terminal output.

Additional: (can be implemented/partially implemented)

  1. Dark mode (turned off by default as this needs some more changes)
  2. Editor and Terminal layouts.

Note: Clicking/selecting any language would open the JavaScript editor and terminal. It requires languages list from backend so for now its fixed for JavaScript. Also the option to switch language dropdown needs data so that is also on hold.

Screenshots:
Home page
image

Search
image

Editor page
image

Looking forward to hearing from you.

@rayhanadev
Copy link

Woah! That's really cool.

@firefish111

This comment has been minimized.

@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa4

This comment has been minimized.

@firefish111

This comment has been minimized.

@inaseem

This comment has been minimized.

@firefish111

This comment has been minimized.

@firefish111

This comment has been minimized.

@inaseem

This comment has been minimized.

@firefish111

This comment has been minimized.

@inaseem

This comment has been minimized.

@raxod502
Copy link
Member

@firefish111 @ch1ck3n-byte Please keep conversations on topic. I've marked your preceding comments as spam, and will report future such comments to GitHub Support.

@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa4

This comment has been minimized.

@firefish111

This comment has been minimized.

@raxod502
Copy link
Member

@ch1ck3n-byte Not to put too fine a point on it, but you posted nine hi-resolution photographs of cows, followed by eight subsequent comments discussing the cows. As far as I can tell, this bears no relationship whatsoever to the pull request under discussion or this repository itself, and as such I consider it off-topic/spam.

@firefish111

This comment has been minimized.

@inaseem
Copy link
Author

inaseem commented Jul 28, 2021

Hey @raxod502 this front end enhancement is currently a separate repo hosted on GitLab. I wanted to get some feedback. Also if this could be replaced with the current front end it will be nice for the end user. Let me know what you think.

@inaseem inaseem closed this as completed Jul 31, 2021
@raxod502
Copy link
Member

Hey @inaseem, I'm sorry for taking so long to get back to you. I've been occupied by trying to get the underlying infrastructure for Riju stabilized under the load of additional users. Now that things are in a pretty good place, I can look at the issue tracker backlog.

As part of the infrastructure stabilization I made it so that LSP isn't enabled until the user explicitly requested it, and this change required some reworking of the frontend to add more interactivity. I took some inspiration from your design when doing this redesign, which you can see live at https://riju.codes/python.

Would you mind sharing a link to the code for your version of the frontend, or making a pull request? I wasn't able to find it.

One thing I wanted to flag was that I would prefer to avoid turning Riju into an SPA or dynamically loading the HTML content. I don't have any objection to adding JavaScript to power things like a language search, but it should remain optional rather than being required simply to load a static list of languages. I think there's a great middle ground between the current state and what you have that would be a big improvement, nonetheless.

@inaseem
Copy link
Author

inaseem commented Sep 15, 2021

Hi @raxod502, thank you for the comment above. I understand that stabilizing the core is the hardest of all.
Great job on doing the hardest parts and the UI improvements. It looks good.

The link that I have shared above was an SPA (create react app). And it uses material-ui (CSS in JS) for all the styling. I understand the concerns around SPA. Also the SPA version takes a while to load the webpage. Keeping those things and new features in mind I have migrated all the existing code to Next JS(server side generation).

I am not creating a PR as of now as I think that there a few backend changes which are also required. The backend changes would mostly be the addition of 2 routes.

  1. getting list of all languages.
  2. getting the details of a single language.

These would be consumed by the Next JS pages on server.

Link for the repo: https://gitlab.com/inaseem/riju

Answers to some expected questions:
Q. Cant we just add CSS to the existing cjs files.
Ans: I am not very familiar working with CJS. Although I tried, but ran into some issues.
Q. Why use Next, React or any front end framework.
Ans: I went through the existing front end code, yes there may not be a need to any of the React/Next etc. Adding React was a personal preference. Your comment above isn't very welcoming to the SPA approach so I found a common ground with Next JS where it's not an SPA and still uses React. Moreover this simplifies the structure of the app and addition of features would be much more simpler.

Please let me know If this is not something which can serve riju's front end requirements.
I also have a few ideas for the editor page which I will try to implement next.

@inaseem inaseem reopened this Sep 15, 2021
@raxod502
Copy link
Member

Thanks for sharing the repo! I looked through the code, and I like that things are split into multiple files. And I've seen in past projects that the plain JavaScript approach can face scalability problems once the UI becomes sufficiently complex, so it seems reasonable to try to mitigate those concerns using React, which I've used with reasonable success in the past.

In light of that, I think this change would be a welcome improvement, as long as it doesn't increase the code complexity too tremendously.

I think that there a few backend changes which are also required

I'm curious about why these are needed. In the current implementation, the server embeds this information (the list of languages, for the index page, and the details of a single language, for the editor page) directly into the JavaScript served to the client:

https://github.com/raxod502/riju/blob/15f959bbeb62b4db985c57a814751f69d880a4a8/frontend/pages/app.ejs#L21-L23
https://github.com/raxod502/riju/blob/15f959bbeb62b4db985c57a814751f69d880a4a8/frontend/pages/index.ejs#L13-L20

This feels better to me since there is no need for another round-trip API call to the server. Does Next.js not support dynamic templating?

The above notwithstanding, per #90 it would actually be better to serve the frontend as a fully static website, which would make it impossible to rely on dynamic templating. So in that case we would need to do things the way you're suggesting anyways.

In light of that, I don't see any reason why these API routes can't be added.

@rayhanadev
Copy link

rayhanadev commented Sep 20, 2021

@radox502

Does Next.js not support dynamic templating?

Next.js is actually serverless, and while you can run a custom server you can't do something like EJS, which you currently have. A small change might have to be made to the API to get that running.

I would think maybe two API routes:

  • /data/languages: fetch metadata about all languages
  • /data/languages/:name: fetch metadata about one language

Then Next.js can use getStaticProps (i.e. fetch the data during build phase) so that the data is baked into the site.

@inaseem
Copy link
Author

inaseem commented Sep 20, 2021

Hey @raxod502 as mentioned by @rayhanadev we can have a custom server, but templating like EJS isn't out of box. For now I have moved the logic to getStaticProps for pre rendering of pages at build time. We only needs the langs object at build time to generate static pages for all languages. These changes are available at https://inaseem.gitlab.io/riju/ with all language links and search working.

The only issue with getStaticProps is that after we have created a build and released, any new additions to langs object would only be generated in a new build. In a way we would have to manually build and release on each new language addition. If we instead fetch the list of languages and language details using an API route on getInitialProps or getServerSideProps then any new additions to langs would not require a rebuild/release of the frontend unless there are front end changes.

@raxod502
Copy link
Member

Yeah, as much as it pains me to make a site dynamically loaded when it could be static, it does make sense to remove the dependency of the frontend on the backend language list.

Having thought it over, I think we should add API routes for fetching the language list, and populate dynamically. We already require JavaScript for the main application to work, so requiring it for the main page I guess isn't the worst thing.

@inaseem
Copy link
Author

inaseem commented Oct 2, 2021

Hi @raxod502 , I have forked and added the changes to front end folder. It's currently in a enhancement/frontend branch. You can try running it locally to test the changes. If you could please create a branch with the same name in your repo, then I will be able to create a pull request on that branch. Which can later merge with master.

Few additional improvements:

  1. Removed setTimeout for reconnecting. If only there is a change in editor then the reconnection will happen which would also update the status to idle->connecting->connected

Here is the link to the same branch on the forked repo. https://github.com/inaseem/riju/tree/enhancement/frontend
Changes can also be seen live here

Instruction for running locally,

  1. yarn build
  2. yarn start

If you want to check the dev build(it runs very slowly). You can simply go with yarn dev

@inaseem
Copy link
Author

inaseem commented Oct 2, 2021

UI improvements on the Editor page(mostly on the controls on top):

image

@inaseem
Copy link
Author

inaseem commented Oct 3, 2021

What's new?

  1. Added Layouts[vertical/horizontal split] for the editor
  2. Redesigned the Navbar
  3. Both the terminal and editor can can be resized using the drag handles[can be spotted in middle]

image

@rayhanadev
Copy link

Love it ❤️

@raxod502
Copy link
Member

raxod502 commented Oct 3, 2021

If you could please create a branch with the same name in your repo, then I will be able to create a pull request on that branch

Hmm, not sure I understand this, you should be able to create a PR just fine regardless of the branch name---I've done so here: #126

Will close this issue so we can continue discussion there.

@raxod502 raxod502 closed this as completed Oct 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants