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

Dev: alternative GUI for Y-axis and X-axis (READY fixed) #1412

Merged
merged 7 commits into from Apr 29, 2020

Conversation

Shnoulle
Copy link
Collaborator

Dev: i send the builded package after, but have error …
Dev: see https://bugs.limesurvey.org/view.php?id=15824#c57383 for sample pic

@Shnoulle
Copy link
Collaborator Author

@thedirtypanda can you help me gain please ?

https://bugs.limesurvey.org/view.php?id=16196

@Shnoulle
Copy link
Collaborator Author

I'm unsure of my commit since i can not really test …

@thedirtypanda
Copy link
Contributor

Try to reinstall the packages in meta lckeditor. But I'm not 100% sure if it would help. Otherwise I will have a look on Wednesday. Tomorrow I'm on vacation.

But I have another question. Why do u want to change it? Cause I already fixed the ticket with X and y axis.

@Shnoulle
Copy link
Collaborator Author

Yes, i see you fix it, but block part are not very clear in my opinion. See https://bugs.limesurvey.org/view.php?id=15824#c57383

  1. Axis are not shown with another color
  2. You put a axis and title on same block : Title are not always just up the title input

And since we already use panel on other part, i think it's cleaner to add it too.

Else : unsure we need to show Y-Axis when there are only one subquestion list.

Capture d’écran du 2020-04-28 09-14-50

VS

Capture d%u2019écran du 2020-04-27 12-37-27

(manually done, but i broke title)
Since it's not an important update, but ergonomy seems best, i try to make a pull request

@Shnoulle
Copy link
Collaborator Author

Shnoulle commented Apr 28, 2020

Try to reinstall the packages in meta lckeditor

You mean assets/packages/editor ?

https://bin.shnoulle.net/?b20c45ac0456fc49#7rX1GbYXOaeo8lhFpjcY69y+o/NIhTUKivDtJOAAgEg=

Seems broken due to NODE_ENV environment variable is undefined. ?

@thedirtypanda
Copy link
Contributor

thedirtypanda commented Apr 28, 2020

No I mean assets/packages/meta lckeditor

@Shnoulle
Copy link
Collaborator Author

I can't build :)

[shnoulle@poledra LsCkeditor]$ yarn
yarn install v1.21.1
[1/4] Resolving packages...
[2/4] Fetching packages...
info fsevents@1.2.9: The platform "linux" is incompatible with this module.
info "fsevents@1.2.9" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
warning " > eslint-plugin-vue@5.2.3" has incorrect peer dependency "eslint@^5.0.0".
warning "eslint-plugin-vue > vue-eslint-parser@5.0.0" has incorrect peer dependency "eslint@^5.0.0".
[4/4] Building fresh packages...
warning Your current version of Yarn is out of date. The latest version is "1.22.4", while you're on "1.21.1".
Done in 17.06s.
[shnoulle@poledra LsCkeditor]$ yarn build
yarn run v1.21.1
$ webpack --mode=production

Insufficient number of arguments or no entry found.
Alternatively, run 'webpack(-cli) --help' for usage info.

Hash: 227491d70522c4cb7c32
Version: webpack 4.41.2
Time: 53ms
Built at: 29/04/2020 09:51:32

ERROR in Entry module not found: Error: Can't resolve '/mnt/data/shnoulle/nginx/www/master/assets/packages/meta/LsCkeditor/src/globalBind.js' in '/mnt/data/shnoulle/nginx/www/master/assets/packages/meta/LsCkeditor'
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@thedirtypanda
Copy link
Contributor

Yeah ... i saw it. Fixed it in latest master. Now it should work. There file was missing. Replaced it with the entrypoint. Just pull latest master.

@Shnoulle
Copy link
Collaborator Author

It build , but my fix didn't fix all :)

I review my part

I have a lot of warning/error when build

Example :

error: 'max' is defined but never used (no-unused-vars) at src/components/subcomponents/_subquestions.vue:311:8:
  309 | 
  310 | <script>
> 311 | import max from 'lodash/max';
      |        ^
  312 | import merge from 'lodash/merge';
  313 | import remove from 'lodash/remove';
  314 | import isEmpty from 'lodash/isEmpty';


error: 'merge' is defined but never used (no-unused-vars) at src/components/subcomponents/_subquestions.vue:312:8:
  310 | <script>
  311 | import max from 'lodash/max';
> 312 | import merge from 'lodash/merge';
      |        ^
  313 | import remove from 'lodash/remove';
  314 | import isEmpty from 'lodash/isEmpty';
  315 | import foreach from 'lodash/forEach';

Maybe we must check in current master ? And fix it ?

It can be a reason question editor broke again and again …

Dev: buil LsCkeditor too , adding translations ?
@thedirtypanda
Copy link
Contributor

Just ignore these warnings. We will remove such unused code, when we are refactoring vue js. Its on the list.

@thedirtypanda
Copy link
Contributor

The Question Editor will break, cause we have the built files inside our repo. Normally this is not the case. We know this issue. And at the moment we are preparing everything for the refactoring of vue js. This will be a deep change and can not be fixed in a quicker way. Just wait.

Dev: More clear text for scale (columns or lines)
@Shnoulle
Copy link
Collaborator Author

Thanks !!

Alt GUI : 2 scales :
Capture d’écran du 2020-04-29 12-16-21

Alt GUI : 1 scale
Capture d’écran du 2020-04-29 12-16-02

:)

maybe @cdorin93 or Frederik can have a look ?

@Shnoulle Shnoulle changed the title Dev: alternative GUI for Y-axis and X-axis Dev: alternative GUI for Y-axis and X-axis (READY fixed) Apr 29, 2020
@Shnoulle
Copy link
Collaborator Author

@maziminke maybe too ?

I think adding columns/lines is more clean too ?

@olleharstedt
Copy link
Contributor

Bug fix or new feature?

@Shnoulle
Copy link
Collaborator Author

Bug is already fixed : adding X-scale and Y-scale

See comment : https://bugs.limesurvey.org/view.php?id=15824#c57383

Neither bug fix or feature, my opinion "Better way to show X-scale and Y-scale"

@thedirtypanda thedirtypanda merged commit aa0cbfc into LimeSurvey:master Apr 29, 2020
@olleharstedt
Copy link
Contributor

🐄

Copy link
Collaborator Author

@Shnoulle Shnoulle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then we show Y-scales (lines) even if "not needed" ?

@Shnoulle Shnoulle deleted the master_altgui_15824 branch April 29, 2020 12:37
@Shnoulle
Copy link
Collaborator Author

@olleharstedt what do you mean about cow ?

PS : it make me learning Vue too, and it's important ;)

@olleharstedt
Copy link
Contributor

Cow is just a joke. Mooo.

@Shnoulle
Copy link
Collaborator Author

Yes, but still : you say it's a lot of discussion for a minimum feature ? 😿

@olleharstedt
Copy link
Contributor

I said this? I have no idea. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants