react-blessed icon indicating copy to clipboard operation
react-blessed copied to clipboard

style instead of class

Open esnunes opened this issue 10 years ago • 17 comments

The current version of react-blessed accepts a class attribute that works in the same way as style of react-native and react itself. I understand the problem of using style because it conflicts with style element attribute however wouldn't be better to support only style and support it as react-native / react?

esnunes avatar Sep 13 '15 22:09 esnunes

Hello @esnunes. You explained quite well the reasons why I went with class instead of style. Not to collide with blessed's already existing namespacing. But my real problem here is that some blessed attributes, which are typically style attributes such as the border type, are not registered under the style property, hence my turmoil.

Yomguithereal avatar Sep 14 '15 08:09 Yomguithereal

That said, I must:

  1. Check whether there is not another blessed way to define border style etc.
  2. Maybe I should create an abstraction over blessed's standard styling so I can include missing items and wrap it in the component magical style attribute.

Yomguithereal avatar Sep 14 '15 08:09 Yomguithereal

Ok, so you cannot, with blessed, register border type through style. So here, maybe it's just a naming problem. The actual classes are like default props you would pass to your component. Do you have a better name, not style nor class then to define it?

Yomguithereal avatar Sep 15 '15 21:09 Yomguithereal

I think in this case it is better to leave the lowercase components as their blessed pair and create upper case ones following react standards. It might be good to do it in as an external library like react-blessed-widgets, what do you think?

esnunes avatar Sep 16 '15 01:09 esnunes

The thing is this solution would be quite heavy compared to the current solution.

Yomguithereal avatar Sep 16 '15 09:09 Yomguithereal

You can think of it like HTML -> React, Blessed -> React. When you create a react (browser) component you do it based on HTML tags, when you create a react (blessed) component you do it based on blessed tags ;)

esnunes avatar Sep 16 '15 12:09 esnunes

I'm curious if you would consider the target consumers of this to be react devs of blessed devs trying out react?

If itis React devs I would argue in favor of using the style and RN approach of building a web style layer into react-blessed, rather than a straight pass-through to the blessed API.

I believe this would lower the barrier to entry and provide the opportunity for a lot of cool things on top of this, such as runtime style validation.

iamdustan avatar Dec 15 '15 01:12 iamdustan

@iamdustan do you mean you would like a css-like interface to blessed style? I am not sure it would be easy nor desirable to implement as this would bloat the lib and hinder performance.

If not, do you mean to say that the current state of style in the lib suits you or should we go in another direction?

Yomguithereal avatar Dec 15 '15 09:12 Yomguithereal

Correct. Doing something like the following quick psuedo-code would be super nice, IMO

<box style={{display: 'flex'}}>
  <box style={{flex: 2, padding: 5}}>
    <PrimaryContentComponent />
  </box>
  <box style={{flex: 1, padding: 5, border: '1px line #eee'}}>
    <SecondaryContentComponent />
  </box>
</box>

It definitely would both increase the size of this and slow down performance, but I think that both are okay for the improved developer experience and runtime validation warnings that could provide.

iamdustan avatar Dec 15 '15 12:12 iamdustan

I'm not sure if it will decrease performance, Vjeux from Facebook developed a javascript version of flexbox which converts to absolute positions (usually absolute positions are faster than calculate everything by hand).

There are some things to address in order to use react-blessed as a production-ready framework (I believe most part of the things are not react-blessed problems but blessed ones). One of the things that will definitely improve user experience is a standard API. Unfortunately blessed API is quite confusing. The node element has a lot of properties and methods although not all "sub-components" implement them, so you never know exactly if it is going to work or not. Blessed usually has more than one way of doing the same thing like setting a property.

I'm not sure how easy is to change blessed (even because of backwards compatibility) but in case blessed can't be drastically changed, I would recommend to do it in react-blessed.

esnunes avatar Dec 15 '15 12:12 esnunes

True. It will definitely increase time inside react-blessed’s diffing/payload creation because of the additional work, but not necessarily any measurable impact on actual rendering time.

I think the note regarding react-blessed problems versus blessed ones is quite relevant. This is arguably one of the reasons why React was created for the browser. Using the DOM APIs directly is difficult to scale and reason about. The DOM also has multiple inconsistent ways of reading and writing attributes or values.

I think react-blessed could and should solve that to be production ready and could have some really cool things built on top of it (two ideas I want: React Blessed Nylas mail client and an IRC replacement written and extendable with React/Blessed).

One last random thought for now is the ability for a great developer experience. I just started looking at react blessed yesterday and warnings, errors, and the like would just disappear. So I added some primitive support for iamdustan/yellowbox-react to react-blessed (only locally for now). Additionally, for react hardware I’m going to be exploring adding support to running the code in a browser session to get full debugging access to things like React’s Developer Tools. I’m hoping to reuse the infrastructure they have in RN for that.

Yellowbox logging

iamdustan avatar Dec 15 '15 14:12 iamdustan

This seems cool indeed. Let me check all those ideas and see whether I can draft something about layout etc. On a side node, I remember that blessed offers a screen.log method that can display message in a dedicated box displayable by pressing F3 (may be another control, I don't remember exactly).

Yomguithereal avatar Dec 15 '15 14:12 Yomguithereal

@iamdustan, were you speaking about this lib for the layout?

Yomguithereal avatar Dec 17 '15 21:12 Yomguithereal

Yes! I have been meaning to explore it after I get a bit more familiar with blessed itself.

Also, needed to turn on debug mode in the blessed constructor and then the button was F12 :)

iamdustan avatar Dec 17 '15 21:12 iamdustan

Well I was not so far from the truth, was I :smile:?

Yomguithereal avatar Dec 17 '15 21:12 Yomguithereal

just to add to this, using a prop of class gets picked up by this eslint rule https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-unknown-property.md

which is enabled by default for jsx linting and therefore in standard and semi-standard linters.

to get around it in standard one would have to comment-ignore every time class is used

davidmarkclements avatar Jan 04 '16 19:01 davidmarkclements

Problem here is really the overlap with blessed style attribute.

Yomguithereal avatar Jan 05 '16 08:01 Yomguithereal