Label Component

[andersevenrud]

Need a "Label" component that could be used in combination with fields with a left/top/bottom arrangement.

[bstee615]

@andersevenrud Hello, I would like to contribute to fix this issue. Let me try to get a better idea of what you are looking for. What I gathered from the description - you are thinking a text tag that will be tied to a field?

[andersevenrud]

Nice!

I was thinking this (illustrating with JSX):

<FieldLabel label="My Input" position="top">
  <TextField />
</FieldLabel>

Which would produce something like:

<div>
  <label>
    <span>My Input</span>
    <div><!--  Child goes here--></div>
  </label>
</div>

Then just have a data- attribute or a classname defined the position of the label.

For assistive technologies both the label and inputs would require for set, but this is a good start.

[bstee615]

Okay, thank you for the tips. This is my first time working on osjs, and there's a lot to take in. It's such a cool project!

Please give me some help on getting things going. I've replaced the default osjs-gui module with my development version and have linked it up with npm link, and as far as I can tell it is building both my development module and the root osjs project.

As far as I can tell, none of my changes are showing up. For example, I changed ToggleField in my source to wrap the radio button in a span rather than a label, but it is still showing up with a label. Any tips? Do I have to run npm link on the dev module every time I build it? I've attached some figures showing the element and my changes - it should be wrapped in a span rather than a label after my update to the source code.

Here is the change I have made to the source:

export const ToggleField = (props = {}, children = []) =>
  createField('toggle-field', props, {
    type: 'checkbox',
    checked: false
  }, (fieldProps) => h('span', { /// Used to be <h('label', ...>

And here, the element highlighted in the inspector should be a span rather than a label.

[andersevenrud]

As far as I can tell, none of my changes are showing up.

Currently all applications have their own @osjs/gui npm module (ES module). So you currently have to run the linking inside an application and then rebuild.

There are some plans of (fixing) changing this: #11

[andersevenrud]

Any luck ? Let me know if you need help 😊

[bstee615]

@andersevenrud I tried linking inside the application (paint-application, in this case), installed in npm-packages/, but no luck. Haven't had any luck yesterday, but I'll try later.

Here's (something like) the sequence of commands I am trying to get changes to show up. I'm not at my PC right now, but basically I'm just building and linking (in order) 1. osjs-gui 2. the whole project, then 3. the paint application.

(in the root directory)
npm --prefix src/osjs-gui build
npm --prefix src/osjs-gui link

npm link @osjs/gui
npm build

npm --prefix npm-packages/@osjs/paint-application link
npm --prefix npm-packages/@osjs/paint-application build

[andersevenrud]

This is usually what I do:

# Libraries
mkdir npm-packages
cd npm-packages
git clone https://github.com/os-js/osjs-gui.git
cd osjs-gui
npm link

# OS.js installation directory
npm link @osjs/gui
cd src/packages
git clone https://github.com/os-js/osjs-draw-application.git
cd osjs-draw-application
npm install
npm link @osjs/gui
npm run build

# Again in OS.js installation directory
npm run package:discover
npm run build

Then after that just run npm run watch:esm in osjs-gui and npm run watch inside the src/packages/osjs-draw-application directory and OS.js install and you should have it working.

The linking inside the OS.js installation directory just ensures that the service provider and stylesheet is loaded. The apps are what imports the GUI components.

[andersevenrud]

Oh, and anything in src/packages will override any npm packages installed.

[andersevenrud]

npm build

Heh, maybe what was missing here is npm run build:esm 😊

The npm run build builds the UMD which is usually only used on codepen and such. I'll update the README for this.

[andersevenrud]

I've updated the README for this.

Also opened an issue to hopefully simplify this process a little bit: #16

[bstee615]

@andersevenrud Ok, I ran the same commands but cannot make any changes in the actual app. I'd really like to contribute, but I don't want to waste your time. I'll work on getting the module to build and link correctly, but you shouldn't depend on me for this Issue request. And honestly, I'm new to server-side JS stuff so the Hyperapp view system here is a little too clever for me at the place I'm at. I will keep trying though, and maybe I can be helpful on future requests.

[andersevenrud]

Ok, I ran the same commands but cannot make any changes in the actual app. I'd really like to contribute, but I don't want to waste your time

No waste, man. I'd want this to work for you, heh.

As long as you watch/build your application in addition to osjs-gui this should work just fine. I'll see if I can't throw together something to demo this :)

And honestly, I'm new to server-side JS stuff so the Hyperapp view system here is a little too clever for me at the place I'm at. I will keep trying though, and maybe I can be helpful on future requests.

All of this is client-side, actually.

[bstee615]

@andersevenrud Oi. Wouldn't you know it, I jinxed myself.

The reason my changes weren't happening was because the properties passed into createField are being filtered. That's why my changes weren't showing up -- I tried a change to Box and it worked. I had made some other changes to test if I was building correctly, but at that point I was using build rather than build:esm.

So, I will get working on this one! Thanks a lot for your patience. I got pretty frustrated, but I'm glad you didn't.

[andersevenrud]

Aha! Well, no wonder then 😅

Looking forward to see what you come up with :)

[bstee615]

@andersevenrud It works! Do you have any styling kind of preferences for the labels such as margins or font size?

So you can review my work before PR: currently, I have diverged from the illustration you recommended. Here is the structure of the component:

<Element class="osjs-gui-field-label osjs-gui-field-label-on-[top, bottom, or left]">
  <label>Label Text</label>
  <Element>
    [Child field element]
  </Element>
</Element>

I did it this way for two reasons: 1. it fits vanilla HTML forms norms and 2. It will allow for better accessibility (I think) when we add the for attribute to the labels, since it's only the label text inside <label>.

<input> tags in Field components don't have id's on them, so I did not proceed with that option for now. I have another unpushed branch that adds that for attribute, but I haven't gone to the work of testing it.

[andersevenrud]

@bstee615 Looking great! 😃

Do you have any styling kind of preferences for the labels such as margins or font size?

The standard margin/padding for pretty much everything is 1/2 em. As for the font style, I'm not really sure. Maybe just bold is enough (or a toggle for it) ?

[bstee615]

@andersevenrud Okay. And lastly, how should I write unit tests? I see that you have the project set up for Jest, but do not see any in the project and package.json does not have tests hooked up.

[andersevenrud]

Glad that you mention it. There's a lack of unit tests across pretty much all packages (embarrassingly).

Though, I'm working on it 🤓 I'm going to commit everything that I've done in @osjs/client as of now (70%+ coverage) and then get started on @osjs/server, @osjs/gui and @osjs/common (in that order).

😅

[andersevenrud]

I actually pushed what I have so far here https://github.com/os-js/osjs-client/tree/unit-tests/ .

So if you want to take a stab at it in this package, basically follow that pattern :)

[andersevenrud]

Completely messed up that branch.... but it should be fixed and up-to-date now.

[bstee615]

@andersevenrud Okay, then I will forgo the unit tests for now and submit a PR.

[andersevenrud]

I just published a new release that contains #17 😄