search

aiidalab / aiidalab-home

AiiDAlab Home App
MIT License
5 stars 9 forks source link

The compatible info only show for installed app #131

Closed unkcpz closed 1 year ago

unkcpz commented 1 year ago

Fixes #100

The compatible info will be shown during the installation, which is not the expected behavior. The compatible info is only shown for the app installed.

danielhollas commented 1 year ago

@unkcpz could you please provide screenshots so we know what this PR is fixing. I am not that familiar with this functionality so screenshots always help a lot.

Otherwise this looks good to me.

unkcpz commented 1 year ago

This PR should go after https://github.com/aiidalab/aiidalab/pull/349, but I just find there is an unexpected behavior of my implementation. I'll summarize both with screenshots. I make it a draft at the moment.

unkcpz commented 1 year ago

Peek 2023-02-09 11-08

Even after the fix of https://github.com/aiidalab/aiidalab/pull/349, from the screenshot you can see during the installation the yellow warning box showing "Reason for incompatibility" will bleak, which should not the case. It is caused from the box is pop out without checking the app is installed.

yakutovicha commented 1 year ago

I just made the PR blocked till we decide whether to go with the sandbox approach or not.

unkcpz commented 1 year ago

After https://github.com/aiidalab/aiidalab/pull/357 change, we are ready with this PR.

unkcpz commented 1 year ago

Sorry, this PR is not needed I think it is all fixed by https://github.com/aiidalab/aiidalab/pull/357. I'll give it a test.

unkcpz commented 1 year ago

Just did the check, this PR is still needed. The changes applied will show the compatibility_info only for the installed app.

@danielhollas can you give this a second look? I think you don't need to test, I test it and I am confident with the issue I show inhttps://github.com/aiidalab/aiidalab-home/pull/131#issuecomment-1423938062 is gone.