D27583: rebuild mosaic multiple dataset

[Deepak Kumar]

dekumar added a comment.


  In D27583#626328 <https://phabricator.kde.org/D27583#626328>, @echarruau wrote:
  
  > In D27583#625178 <https://phabricator.kde.org/D27583#625178>, @jjazeix wrote:
  >
  > > In D27583#625177 <https://phabricator.kde.org/D27583#625177>, @dekumar wrote:
  > >
  > > > In D27583#624933 <https://phabricator.kde.org/D27583#624933>, @jjazeix wrote:
  > > >
  > > > > I'm wondering if we should not have the image names in the dataset too so later we can choose which images to display
  > > >
  > > >
  > > > Can you elaborate a bit more about it? Are you talking about the images in  mosaic.js file?
  > >
  > >
  > > Yes, should we allow the possibility to the creator of the dataset to specify which images he wants? May be useful later for teachers or customisation. May be too much too.
  > >  @timotheegiet @echarruau does it make sense?
  >
  >
  > Hi,
  >  It does really make sens. We  need to think about the "step after". We may have in mind, (even if we do not do it at the end) that our server should allow to customise activities. The server could allow adding sublevels then presenting the attributes to fill. The variable therefore must be self explanatory, and we may have to add comments in front of the attributes to tell what they are and how to fill them. eg: //layout: describes the way the cells are displayed, eg: "layout": [ [6,4], [12,2] ],
  >  In my case as programmer, thinking about the final user would force me to replace for example nbItems by nbOfCells which is clearer for the final user.
  >  Here as final user, for example I do not know what multipleDatasetType is neither multipleDataset2.
  
  
  @echarruau  Hi, Thanks for the review. I would rename the variable with good names and add comments too as described by you.
  
  In D27583#626328 <https://phabricator.kde.org/D27583#626328>, @echarruau wrote:
  
  > In D27583#625178 <https://phabricator.kde.org/D27583#625178>, @jjazeix wrote:
  >
  > > In D27583#625177 <https://phabricator.kde.org/D27583#625177>, @dekumar wrote:
  > >
  > > > In D27583#624933 <https://phabricator.kde.org/D27583#624933>, @jjazeix wrote:
  > > >
  > > > > I'm wondering if we should not have the image names in the dataset too so later we can choose which images to display
  > > >
  > > >
  > > > Can you elaborate a bit more about it? Are you talking about the images in  mosaic.js file?
  > >
  > >
  > > Yes, should we allow the possibility to the creator of the dataset to specify which images he wants? May be useful later for teachers or customisation. May be too much too.
  > >  @timotheegiet @echarruau does it make sense?
  >
  >
  > Hi,
  >  It does really make sens. We  need to think about the "step after". We may have in mind, (even if we do not do it at the end) that our server should allow to customise activities. The server could allow adding sublevels then presenting the attributes to fill. The variable therefore must be self explanatory, and we may have to add comments in front of the attributes to tell what they are and how to fill them. eg: //layout: describes the way the cells are displayed, eg: "layout": [ [6,4], [12,2] ],
  >  In my case as programmer, thinking about the final user would force me to replace for example nbItems by nbOfCells which is clearer for the final user.
  >  Here as final user, for example I do not know what multipleDatasetType is neither multipleDataset2.
  
  
  Hello @echarurau, Thanks for the review. I would rename the variables with good name and add comments too.
  
  In D27583#626328 <https://phabricator.kde.org/D27583#626328>, @echarruau wrote:
  
  > In D27583#625178 <https://phabricator.kde.org/D27583#625178>, @jjazeix wrote:
  >
  > > In D27583#625177 <https://phabricator.kde.org/D27583#625177>, @dekumar wrote:
  > >
  > > > In D27583#624933 <https://phabricator.kde.org/D27583#624933>, @jjazeix wrote:
  > > >
  > > > > I'm wondering if we should not have the image names in the dataset too so later we can choose which images to display
  > > >
  > > >
  > > > Can you elaborate a bit more about it? Are you talking about the images in  mosaic.js file?
  > >
  > >
  > > Yes, should we allow the possibility to the creator of the dataset to specify which images he wants? May be useful later for teachers or customisation. May be too much too.
  > >  @timotheegiet @echarruau does it make sense?
  >
  >
  > Hi,
  >  It does really make sens. We  need to think about the "step after". We may have in mind, (even if we do not do it at the end) that our server should allow to customise activities. The server could allow adding sublevels then presenting the attributes to fill. The variable therefore must be self explanatory, and we may have to add comments in front of the attributes to tell what they are and how to fill them. eg: //layout: describes the way the cells are displayed, eg: "layout": [ [6,4], [12,2] ],
  >  In my case as programmer, thinking about the final user would force me to replace for example nbItems by nbOfCells which is clearer for the final user.
  >  Here as final user, for example I do not know what multipleDatasetType is neither multipleDataset2.

INLINE COMMENTS

> echarruau wrote in Mosaic.qml:127
> I don't understand this multipledataset1 variable. 
> Is it related to the device layout (portrait/landscape) or to the layout targeted 'single/multiple line".

@echarruau Hi, It is related with the targeted layout "single/multipleLine". I have made the use the states in the code to change different property in case of both of the multiple dataset.

> echarruau wrote in Data.qml:24
> "Rebuild" not rebuid. Anyway the we could have a shorter description:
> Items are on a single line.
> Items on multiple lines.

Sure, I would update it.

REPOSITORY
  R2 GCompris

REVISION DETAIL
  https://phabricator.kde.org/D27583

To: dekumar, #gcompris_improvements, jjazeix, timotheegiet, echarruau
Cc: kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20200312/12c11337/attachment-0001.html>