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

List of proposals for improvement #67

Open
6 of 12 tasks
aralroca opened this issue Dec 7, 2020 · 5 comments
Open
6 of 12 tasks

List of proposals for improvement #67

aralroca opened this issue Dec 7, 2020 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@aralroca
Copy link
Collaborator

aralroca commented Dec 7, 2020

After a review, I add a list of proposals for improvement:

  • Provide directly the useTraversal hook. It is not recommended that the libraries directly expose the context and users have to write useContext(TraversalContext).
  • Consistency between the Form elements. Input, TextArea, FileUpload etc each with its own properties which are different from the standard HTML5. Or they are the same as HTML5 or they should have their own consistent version across all elements. For example: onChange now sometimes receives the value and other times the event.
  • Add source-maps to make it easier to debug for those who use the library.
  • Deprecate Ctx.filterTabs and instead use TabPanel props. Or keep both, but I would make the TabPanel have this controlled by default.

Instead of:

const calculated = ctx.filterTabs(tabs, tabsPermissions);
return <TabsPanel tabs={calculated} {...props} />

Do:

return <TabsPanel tabs={tabs} tabsPermissions={tabsPermissions} {...props} />
  • Change the currentTab prop of the TabPanel behavior. If the currentTab doesn't match with any tab that the user has permissions, then the first tab should be selected.
  • Add types to every configuration and context content. We don't need to convert the project to TypeScript but this way, those who use our API from the IDE itself can know, for example, when doAction what actions are available without having to consult our documentation. 
  • Add a formatter like Prettier to keep the code in a certain order.
  • Add missing files like CONTRIBUTING.md, LICENSE...
  • Remove linkstate dependency (only used in 1 component)
  • Replace react-use to own hooks: It's importing 60kb for 3 simple hooks...
  • Add an example in the repo of a simple admin. This will be useful for two things:
    • Make it easier for users to get started
    • Allow us to test the basic functionalities, either with cypress or in the development environment
  • ... Anything else you would like to add?
@aralroca aralroca added the enhancement New feature or request label Dec 7, 2020
@masipcat
Copy link
Collaborator

masipcat commented Dec 9, 2020

Before commenting about the list of proposal I'd like to ask this question:

What's the purpose of this project? It's clear to me that this project is (at least) a management interface (aka Admin UI) but I also think this project could be lib/client for Guillotina. That's what someone could think looking at the repo name guillotina_react (like me :P). Also, I'm not sure if we want the AdminUI to do a lot of things or something very simple and small that's just a foundation to build things on top of it.

If we want AdminUI to do a lot of things OOTB, maybe it's interesting to move to antd (39.4MB) but if we want something minimal, I'd stay with bulma (2.33 MB). Also if we consider this project as a lib/client/sdk, maybe there is a way to "split" the project in two layers: core and UI. Anyway, I'd like to discuss and define what we want and then we can discuss some improvements.

But right now, I'm ok with adding a CONTRIB, LICENSE, Prettier and adding consistency to the Form elements.

@jordic
Copy link
Collaborator

jordic commented Dec 10, 2020

I agre with @masipcat opinion. The main idea behind guillotina_react is to provide a minimal package that can provide whatever needed to work with a guillotina, with as less as possible dependecies.

Another idea on my head that I will try to fix soon, is to bundle guillotina_react as an es module, that we can import from regular htmls.

@rboixaderg
Copy link
Contributor

It will be interesting to isolate client in a new context and hook to allow use client in a project without traversal, or in a isolated pages outside traversal?

@jordic
Copy link
Collaborator

jordic commented Jan 2, 2021

@rboixaderg sounds interesting :)

@rboixaderg rboixaderg linked a pull request Jan 9, 2021 that will close this issue
@rboixaderg rboixaderg removed a link to a pull request Jan 9, 2021
@rboixaderg
Copy link
Contributor

@aralroca @jordic what would be the best way to introduce translations into the project? A custom implementation or a library ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants