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

Unify React component with HTML template - discussion #265

Closed
magicmatatjahu opened this issue Mar 17, 2021 · 21 comments
Closed

Unify React component with HTML template - discussion #265

magicmatatjahu opened this issue Mar 17, 2021 · 21 comments
Labels
area/library Related to all activities around Library package discussion enhancement New feature or request stale
Milestone

Comments

@magicmatatjahu
Copy link
Member

magicmatatjahu commented Mar 17, 2021

We're currently maintaining two HTML documentation views: the HTML template and this React component. This is a big problem, because we must maintain two source code, which are do this same - render AsyncAPI spec as HTML. Of course users can use React component in SPA application and for dynamic data, and HTML template renders static content, but purpose is this same.

Some advantages with unification:

  1. Less code to maintain.
  2. We have 3 playground - playground for React component, playground for generator and new studio which is in early dev stages. After this task, we will replace these 3 playground only with one - Studio. (go to 1. point). Of course replacing all playgrounds is not in scope of unification but probably most important part for followup PRs,
  3. Unificated styles across official ecosystem of AsyncAPI - in website, Studio and React component. React component uses by default Fiori styling, but Studio and website use TailwindCSS.

I think that 1. and 2. points for everyone is very good and no one should have a problem with it (if you have, please write about your point of view). The most problematic is point 3. At the moment we have "theming" feature inside React component. User can override styling by overriding styles for appropriate css classes in this file. We know that some companies/users replace default styling with new, related to internal design system of company etc, but we wanna know what impact will it have on your products that we will change the default styling and remove currently "theming" system for the next 2 months? Our priority is to unify styles and code across the official tooling ecosystem, and then add features to the component, such as theming or plugins. Please share your thoughts! The way of rendering of some parts will also change - for example for JSON Schema.

NOTE: We will use TailwindCSS, but we have plan to make our "styling" framework (probably based on TailwindCSS), but this only an idea, and we don't know when it will be done.

Disadvantages:

For active HTML template's contributors:

  • What do you think about this movement? Do you feel good that for contribution to the next versions of HTML template, you will write in React? Of course your contributions won't be removed 😄 but the "way" to add new contributions. To be honest... your code will be removed, I mean whole Nunjucks templates and for someone it can be a problem. Please share your thoughts!

For more info of each step of unification, please check tasks:

@magicmatatjahu magicmatatjahu added enhancement New feature or request area/library Related to all activities around Library package discussion labels Mar 17, 2021
@magicmatatjahu magicmatatjahu pinned this issue Mar 17, 2021
@derberg
Copy link
Member

derberg commented Mar 17, 2021

just a small comment about react playground, that I would not remove it as it helps with development, unless we find a way to test the component easily against the studio.

would be great if you could not only clearly list advantages but disadvantages too. I'm not sure if people are aware that new HTML template after the change will not be "lightweight" any more, react or react as web component are not lightweight. I personally do not see it as a problem, as this is what browser cache is for, but anyway, should be mentioned.

or is it actually enough I wrote it here 😄

@magicmatatjahu
Copy link
Member Author

@derberg

just a small comment about react playground, that I would not remove it as it helps with development, unless we find a way to test the component easily against the studio.

Of course! I agree with you, I should be more clarify in this topic. I wanna remove playground when we switch to Studio and find some way to use Studio as standalone developer tool to "test" React component.

would be great if you could not only clearly list advantages but disadvantages too. I'm not sure if people are aware that new HTML template after the change will not be "lightweight" any more, react or react as web component are not lightweight. I personally do not see it as a problem, as this is what browser cache is for, but anyway, should be mentioned.

Yep, it's not lightweight (I will mention about it), but problem is mostly related to parser-js not to component itself, you can see what "exactly" we have in bundled component here - packages related to dereferencing, ajv, yaml-ast etc is about 1 mb versus about 200kb for pure React component with his dependencies. I have idea to split parser-js package to 2 packages with validation + models and one only with models - in some situation you wanna validate schema in server and then send only parsed schema to component - for this situation you don't need this boilerplate with modules with ajv etc inside component.

@derberg
Copy link
Member

derberg commented Mar 17, 2021

awesome.

and yes, we need to check if we can get that beast slimmer, different topic though, and I'm not 100% sure parser is guilty here although I see it is 724KB large already. Different subject, just wanted to make it clear to others

@lornajane
Copy link

Will the new tools be WCAG compliant, or do we have other accessibility and inclusion goals in mind? I personally would rather have a lightweight HTML reference documentation output format than what sounds like a splendid and fully-featured client-side application - but more than that, many organisations have a legal obligation to comply with basic accessibility, so we should make sure that we're making something that even those organisations can adopt.

As a community project, to include as many users as we can, and as many organisations as we can, would be in line with our values. It's easier when things are built well from the beginning, so can we take this opportunity to show our commitment to inclusion of all sorts of participants?

@leonheess
Copy link

We are using the web component version in Vue.js with custom styling. Unfortunately, that's currently pretty cumbersome - would that become better or worse with this unification?

@magicmatatjahu
Copy link
Member Author

@lornajane Hello! I know about this problem and I mentioned it in this issue asyncapi/shape-up-process#83 Unfornatelly as you know, HTML Template doesn't support a11y - only in menu, React component also doesn't, but if we leave HTML Template and React as separate "projects" then we will have much more code to maintance and with this also will be poorer accessibility - we will have to ask contributor to change two projects with a11y in mind. Of course we will have in mind your suggestions on "rewriting" component :)

@leonheess Hello! What do you have in mind with Unfortunately, that's currently pretty cumbersome? Do you mean "theming" or integrating component in Vue application hasn't good DX?

@leonheess
Copy link

leonheess commented Mar 18, 2021

@magicmatatjahu Mainly theming, most web components export their default stylesheet so you can import it into your style sheet like @import '~@asyncapi/web-component/style.css'; and that allows you to define new rules below which overwrite the imported ones if necessary. With your way of using a prop like cssImportPath it requires us to have a fairly complex webpack configuration that bundles the CSS file in a very specific way to be able to export our application as a web component and still have an import path for your component.

However, our main concern with this issue is: Will the web component still be available in the same way after the proposed change?

@magicmatatjahu
Copy link
Member Author

@leonheess Thanks for response! I see your PoV on theming and I agree with you, at the moment it's not a good DX. We must address this issue. Now we want to focus on unification, and then on "theming" and plugins. As for your second question. Don't worry, the web-component will still be the same, with this same props, only its default styling, which is currently Fiori based, will be changed to Tailwind css. Also some parts could be change, I mean "Servers" will be rendered before "Channels", we will render CorrelationID and other missed parts in component etc.

If your product allows it, are you able to make a printscreen (e.g. toggled channel) what does your component styling currently look like? It will be helpful for us, to show how users use our component :)

@leonheess
Copy link

@magicmatatjahu Thanks a lot, I will check if we can share a screenshot and come back to you

@fmvilas
Copy link
Member

fmvilas commented Apr 12, 2021

Agree with @magicmatatjahu. @leonheess it would be super helpful for us to get a screenshot or demo of your styling now that we're working on it 🙏

@magicmatatjahu
Copy link
Member Author

Hello everyone! We published the 1.0.0-next.1 version of component with some breaking changes. You can read about them here #327 If you have any questions, feel free to ask. Here is also a branch with next version https://github.com/asyncapi/asyncapi-react/tree/next with updated docs. We want to focus on next branch and in master only fixes very bothersome bugs.

@leonheess
Copy link

leonheess commented May 12, 2021

@magicmatatjahu Sorry for the delay - it took some time to get approval:

screenshot

@magicmatatjahu
Copy link
Member Author

@leonheess You don't need to apologize :) Component is for community, not for us :) Thanks you for your work. I really appreciate it :) When we will re-create theming, we will remember about such a re-writing styling.

@magicmatatjahu magicmatatjahu added this to the Road to 1.0.0 milestone Jul 7, 2021
@fmvilas
Copy link
Member

fmvilas commented Jul 15, 2021

Is there something missing from that issue?

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 60 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jan 14, 2022
@derberg
Copy link
Member

derberg commented Jan 14, 2022

I think we can close this one @magicmatatjahu

@github-actions github-actions bot removed the stale label Jan 15, 2022
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added stale and removed stale labels Sep 14, 2022
@fmvilas
Copy link
Member

fmvilas commented Sep 16, 2022

@magicmatatjahu isn't this issue solved already? 🤔 Can we close it?

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity 😴

It will be closed in 120 days if no further activity occurs. To unstale this issue, add a comment with a detailed explanation.

There can be many reasons why some specific issue has no activity. The most probable cause is lack of time, not lack of interest. AsyncAPI Initiative is a Linux Foundation project not owned by a single for-profit company. It is a community-driven initiative ruled under open governance model.

Let us figure out together how to push this issue forward. Connect with us through one of many communication channels we established here.

Thank you for your patience ❤️

@github-actions github-actions bot added the stale label Jan 15, 2023
@fmvilas fmvilas closed this as completed Mar 29, 2023
@derberg derberg unpinned this issue Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/library Related to all activities around Library package discussion enhancement New feature or request stale
Projects
None yet
Development

No branches or pull requests

5 participants