os-js / osjs-client

OS.js Client Module
https://manual.os-js.org/
Other
31 stars 31 forks source link

osjs basic-application #146

Closed mahsashadi closed 3 years ago

mahsashadi commented 3 years ago

Hi I HAVE item value defined in below map function. How can I access item in 'open-file' action of a basic osjs application?

import React, { useState, useEffect } from 'react';
import useCore from './hooks/core';

const fileNames = ['file1', 'file2', 'file3', 'file4', 'file5', 'file6', 'file7'];

export default function Security(props) {
   // basic is created by `core.make('osjs/basic-application', proc, win)`
    const {basic} = useCore();
    const [fileData, setFileData] = useState();

    useEffect(() => {
        basic.on('open-file', (selectedData) => {
            setFileData(selectedData)
           // I NEED TO ACCESS ITEM HERE
            setFileData({...fileData, item: selectedData.path} )
        });
    }, []);

    return(
        <div>
            {fileNames.map( (item) =>(
                <div>
                    <label>{item}:</label>
                    <input key= {item} value= {fileData.item}></input>]
                    <button onClick={()=>  basic.createOpenDialog()}>Browse</button>
                </div>
            ))}
        </div>
    )
}
andersevenrud commented 3 years ago

Sorry, but I'm not really sure what the question is here. The first argument of a open-file signal is the item :thinking:

andersevenrud commented 3 years ago

Oh, I misinterpreted your code...

If you want to have access to the item, you might have to do something like this:

const [currentItem, setCurrentItem] = useState();

const onBrowse = item => () => {
  setCurrentItem(item)
  basic.createOpenDialog()
}

useEffect(() => {
    basic.on('open-file', (selectedData) => {
      console.log(currentItem)
    });
}, []);
<button onClick={onBrowse(item)}>Browse</button>
mahsashadi commented 3 years ago

Oh!Thanks, I was describing again but it seems you understand my meaning.

Exactly I have tried setting a currentItem as a state, but ( due to react policies for updating states) it is not updated till I call createOpenDialog()

So I was thinking if there was a way to handle that via passing parameters to basic-application functions

andersevenrud commented 3 years ago

It does not even work when adding currentItem to the array on the useEffect ? `useEffect(() => {}, [currentItem]);

andersevenrud commented 3 years ago

Just a small tip when using events like this. You should unregister them with the component lifecycle:

const onSomething = () => {}
useEffect(() => {
  foo.on('something', onSomething)
  return () => {
    foo.off('something', onSomething)
  }
}, [onSomething])
mahsashadi commented 3 years ago

It does not even work when adding currentItem to the array on the useEffect ? `useEffect(() => {}, [currentItem]);

yes, it does not work (adding state to this array means do whatever is in useEffect when that state changes, but my problem is changing that state itself)

andersevenrud commented 3 years ago

Ah. I'm a bit rusty when it comes to React :sweat_smile:

I think I might approach this in a slightly different way :thinking:

Maybe something like this:

const register = (core, args, options, metadata) => {
  const proc = core.make('osjs/application', {args, options, metadata});
  const win = proc.createWindow({});
  const basic = core.make('osjs/basic-application', proc, win, {
    defaultFilename: 'New File.txt'
  });

  proc.on('open-file', (item) => {
    basic.off('open-file');
    basic.once('open-file', (selectedData) => {
      proc.emit('open-file-result', item, selectedData);
    });
  });

  win.render();
  basic.init();

  return proc;
};

This way you have all of this logic isolated and you just have to proc.emit('open-file', item) to get the dialog and proc.on('open-file-result', (item, selected) => {}) to catch the results you want.

andersevenrud commented 3 years ago

You probably don't need to do the basic.off('open-file') if you make sure it's only called once -- but I just placed it there to make sure it's only usable from one place at a time.

mahsashadi commented 3 years ago

This way you have all of this logic isolated and you just have to proc.emit('open-file', item) to get the dialog and proc.on('open-file-result', (item, selected) => {}) to catch the results you want.

Yes, exactly what I was looking for, thanks :+1:

But I still have some problems, although I am trying to update my state as I have always done, It is not going to update correctly! Is there any conflict with doing that in the open event listener? (only the last chosen pair of key value is saved in the whole fileData object)

import React, { useState, useEffect } from 'react';
import useCore from './../../hooks/core';

const fileNames = ['file1', 'file2', 'file3'];
const filedata = {
    'file1':' ',
    'file2':' ',
    'file3':' ',
}

export default function Security(props) {
    const {proc, vfs, basic} = useCore();
    const [fileData, setFileData] = React.useState(filedata);

    useEffect(() => {
        proc.on('open-file-result', (item, selected) => {
            setFileData({
                ...fileData,
                [item]: selected.path
            })
        });

        return ()=>{
            proc.off('open-file-result', (item, selected) => {
                setFileData({
                    ...fileData,
                    [item]: selected.path
                })
            });
        }
    }, []);

    const onBrowse = item => () => {
        proc.emit('open-file', item)
        basic.createOpenDialog()
      }

    return(
        <div>
            {fileNames.map( (item) =>(
                 <div className='groupContainer_grid3'>
                    <label>{item}:</label>
                    <input key= {item} value= {fileData[item]}></input>
                    <button type='button' onClick={onBrowse(item)}>Browse</button>
                </div>
            ))}
        </div>
    )
}
andersevenrud commented 3 years ago

I can't see anything obviously wrong, but your event unregistration will not work as you expect. You have to pass the function as reference!

const onFileResult = (item, selected) => {
  setFileData({
      ...fileData,
      [item]: selected.path
  })
}

useEffect(() => {
    proc.on('open-file-result', onFileResult)

    return ()=>{
      proc.off('open-file-result', onFileResult)
    }
}, [onFileResult]);

only the last chosen pair of key value is saved in the whole fileData object

I'm not 100% sure what you mean by this. Can you give an example ?

mahsashadi commented 3 years ago

but your event unregistration will not work as you expect. You have to pass the function as reference!

Thanks :+1:

Can you give an example ?

Imagine I have two input boxes (must be filled by chosen file path) and two corresponding browse button

//after opening file in the open dialog for the first time, the value of the corresponding key is correctly changed : 
fileData:{
  file1: 'osjs:/image1.png',
  file2: ' '
}

//but by opening file in the open dialog for the next times, only the last value will be changed and the file1 value will be back to initial state like below:
fileData:{
  file1: ' ',
  file2: 'osjs:/image2.png ',
}
andersevenrud commented 3 years ago

Strange. I can't see anything wrong with how you handle the state there.

The only thing that comes to mind is that maybe somehow the remplate is re-rendered, making it so this component is re-built rather than updated.

mahsashadi commented 3 years ago

Strange. I can't see anything wrong with how you handle the state there.

yes, I do the same for another component with same structure, but I do not handle updating state inside event listener like this ... and it works fine!

The only thing that comes to mind is that maybe somehow the remplate is re-rendered, making it so this component is re-built rather than updated.

No, I checked and that's not the problem, I even check handling state outside in a parent component and sending the setFileData as a prop. Same things happened!

andersevenrud commented 3 years ago

yes, I do the same for another component with same structure, but I do not handle updating state inside event listener like this ... and it works fine!

The event listening should not really matter :thinking:

No, I checked and that's not the problem

Well, then I'm not really sure. Another thing that comes to mind is maybe usage of keys on components. My gut is telling me that this is a symptom of something else (mainly re-rendering).

I also thought maybe you had a spelling error (filedata vs fileData), but if your pasted code is what you're using that does not seem to be the case.

mahsashadi commented 3 years ago
 const [fileData, setFileData] = React.useState(filedata);

My problem is solved by removing the initial value of state by an empty object React.useState({}). however I did not understand what was wrong about that!!

Anyway I thank you a lot for looking at my code :)

andersevenrud commented 3 years ago

Ah. Great :) It might be passed as reference. Try useState({ ...filedata }).

Just a note: Please close your own issues when they are resolved.