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

Make plugins/editors show one entry for each plugin with a drop down for version #13942

Merged
merged 3 commits into from
Aug 16, 2019

Conversation

JPinkney
Copy link
Contributor

What does this PR do?

This PR introduces the functionality and styling for having plugins/editors show one entry for each plugin, with a drop down to select the version. Previously, the plugins page and the editors page showed an entry for every version of the plugin.

Video: https://www.youtube.com/watch?v=WkrbTsrjuBk

What issues does this PR fix or reference?

#13566

Release Notes

Modified plugin and editor dashboard pages to have one entry for each plugin with a drop down to select the version

@che-bot
Copy link
Contributor

che-bot commented Jul 22, 2019

Can one of the admins verify this patch?

1 similar comment
@che-bot
Copy link
Contributor

che-bot commented Jul 22, 2019

Can one of the admins verify this patch?

…versions

Signed-off-by: Josh Pinkney <joshpinkney@gmail.com>
@musienko-maxim
Copy link
Contributor

Can one of the admins verify this PR?

@l0rd
Copy link
Contributor

l0rd commented Jul 22, 2019

@JPinkney thanks!

@ashumilova @rhopp can you please have a look. If you consider that it's safe enough I would rather be +1 to merge this PR because it has an important impact on UX (even if it isn't inclued in the endgame).

@slemeur
Copy link
Contributor

slemeur commented Jul 23, 2019

Nice work @JPinkney !

Ping @ashumilova and @rhopp . That is something we would like to integrate in 7.0.0

@ashumilova
Copy link
Contributor

Reviewed and like the proposed changes and +1 for merging.
One thing discovered and need to be fixed - when workspace is updated in devfile editor and saved, than the state of plugins and editors pages is not consistent, until user refreshes the page.

@benoitf
Copy link
Contributor

benoitf commented Jul 23, 2019

I would not merge it, it's an enhancement, linked issue is not event tagged for 7.0.0

@nickboldt nickboldt added the kind/enhancement A feature request - must adhere to the feature request template. label Jul 23, 2019
Copy link
Contributor

@nickboldt nickboldt left a comment

Choose a reason for hiding this comment

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

So recapping that's a +1 from Anna (with a requested change) and a -1 from Florent.

I suppose @rhopp is the tiebreaker here?

Unclear why I'm being asked to PR review on this one, but here we go:

  • code: +0, I'm not an expert
  • concept:+1, makes UI cleaner and appears to sync between the plugins and devfile tabs/content.
  • QE: -0.5: might break existing UI tests, but not my call.

@rhopp
Copy link
Contributor

rhopp commented Jul 24, 2019

ci-test

@che-bot
Copy link
Contributor

che-bot commented Jul 24, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:13942
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@vitaliy-guliy
Copy link
Contributor

vitaliy-guliy commented Jul 24, 2019

I would display the version as a plain text and display the select field only when hovering the row.
@JPinkney could you please try to play with it. You would like this behavior.

@slemeur
Copy link
Contributor

slemeur commented Jul 26, 2019

I like @JPinkney proposal as it is. If the select box was appearing only on hover, I would not understand that I can change the version. Here it is clear IMO.

+1 to merge for 7.0.0 if @rhopp and @ashumilova are fine too.

Copy link
Contributor

@benoitf benoitf left a comment

Choose a reason for hiding this comment

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

target was 7.1.0 but it has been changed to 7.0.0 so I'm fine

@ashumilova
Copy link
Contributor

yeah, but need to fix the way how plugins and editors pages are updated on changes in devfile (in editor).

@ashumilova
Copy link
Contributor

ashumilova commented Aug 8, 2019

When we do updates in Yaml editor on "Devfile" page of workspace details, the following method is called:
https://github.com/eclipse/che/blob/d89cf6f0c3b28671cfcef1cfbc4344ca4d110d1d/dashboard/src/app/workspaces/workspace-details/workspace-plugins/workspace-plugins.controller.ts#L71

it has a check, whether plugin is selected (isPluginEnabled).
If we had plugin1/version1 selected, and user changed in editor to plugin1/version2, the check works wrong in that case. We need to find plugin id without version and if there are version changes - keep the item selected with the changed version.

@che-bot che-bot added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Aug 8, 2019
@JPinkney
Copy link
Contributor Author

JPinkney commented Aug 8, 2019

I'll try to fix that as soon as possible but I'm currently working on the last blocker for Che 7 (#13427) so I can't give any accurate timeline on when it will be done

@akurinnoy
Copy link
Contributor

@JPinkney seems there is minor fix so I could prepare a commit with fix. You'll only need to test it and merge, wdyt?

@JPinkney
Copy link
Contributor Author

JPinkney commented Aug 8, 2019

@akurinnoy Yeah that would be great!

Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
@akurinnoy
Copy link
Contributor

ci-test

@che-bot
Copy link
Contributor

che-bot commented Aug 9, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@akurinnoy
Copy link
Contributor

@rhopp I've just added commit which fixes bug mentioned here

It seems happy-path-tests are not happy with that. Is this fail related to my commit? Is there something I could do to fix it?

@dmytro-ndp
Copy link
Contributor

dmytro-ndp commented Aug 9, 2019

@akurinnoy: PR check has been failed because it uses Jenkinsfile now, which has been merged in master tonight. So, it requires to take changes from master.

@eclipse-che eclipse-che deleted a comment from che-bot Aug 9, 2019
@dmytro-ndp
Copy link
Contributor

ci-test

@che-bot
Copy link
Contributor

che-bot commented Aug 9, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@che-bot
Copy link
Contributor

che-bot commented Aug 9, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:13942
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@che-bot
Copy link
Contributor

che-bot commented Aug 9, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@dmytro-ndp
Copy link
Contributor

ci-test

@che-bot
Copy link
Contributor

che-bot commented Aug 9, 2019

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:13942
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

@JPinkney
Copy link
Contributor Author

JPinkney commented Aug 9, 2019

@akurinnoy I've just tested out your fix and it seems like everything is working!

@che-bot
Copy link
Contributor

che-bot commented Aug 9, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@che-bot
Copy link
Contributor

che-bot commented Aug 9, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:13942

@akurinnoy
Copy link
Contributor

ci-build

@akurinnoy
Copy link
Contributor

@rhopp please have a look. Can we merge this PR?

@rhopp
Copy link
Contributor

rhopp commented Aug 15, 2019

Looking good with one small drawback with the scrolling:
ezgif-4-1a4123601680
I'm using Google Chrome 76.0.3809.87.

@akurinnoy
Copy link
Contributor

@rhopp I'm sure issue with scrolling is not related to this changes. Please file an issue for this bug.

@amisevsk
Copy link
Contributor

@rhopp @akurinnoy I can confirm that the scrolling issue is present most versions of Che, I've opened issue #14256

@akurinnoy akurinnoy merged commit 87d4a10 into eclipse-che:master Aug 16, 2019
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Aug 16, 2019
@che-bot che-bot added this to the 7.1.0 milestone Aug 16, 2019
skabashnyuk pushed a commit that referenced this pull request Jan 3, 2020
Make plugins/editors show one entry for each plugin with a drop down for version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.