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

Move login to menu content #1056

Merged
merged 1 commit into from
Dec 9, 2024
Merged

Conversation

amvanbaren
Copy link
Contributor

@amvanbaren amvanbaren commented Nov 29, 2024

Makes it possible to make the webui read only as requested in #1041.
The default menu stays the same. The mobile menu is a bit different.

BEFORE AFTER

@amvanbaren amvanbaren self-assigned this Nov 29, 2024
@amvanbaren
Copy link
Contributor Author

@kineticsquid @filiptronicek What do you think?

@kineticsquid
Copy link
Contributor

I'm good with the requirement to make the webui read only. This PR looks good to me. I'd like to hear from @filiptronicek and @spoenemann.

Copy link
Member

@filiptronicek filiptronicek left a comment

Choose a reason for hiding this comment

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

Changes look good, the mobile menu improvements are very appreciated.

Makes it possible to make the webui read only as requested in #1041.

Could you point me to the read-only changes in the PR? I couldn't find anything related to it.

@svor
Copy link
Contributor

svor commented Dec 3, 2024

Hello @amvanbaren, @filiptronicek!
Thank you for looking at my request! I was working a bit on #1041 as well and it's what I have: #1041
Could you please take a look if it does make sense.

@amvanbaren
Copy link
Contributor Author

@filiptronicek @svor It makes the webui read only in the sense that you can exclude the publish and login buttons from the menu content.

The open-vsx.org repo shows how to customize the menu content:
https://github.com/EclipseFdn/open-vsx.org/blob/43c654cf6e567bb720749d4323996bc23598e81b/website/src/page-settings.tsx#L190-L191
https://github.com/EclipseFdn/open-vsx.org/blob/main/website/src/menu-content.tsx

@svor
Copy link
Contributor

svor commented Dec 5, 2024

@amvanbaren thank you, this looks great! However, in the case of a private OpenVSX registry, I would prefer not to add a dummy OAuth2 configuration in the application.yaml. Instead, it would be more convenient to simply omit the configuration. In this scenario, the Publish and Login buttons would not be displayed in the UI by default, eliminating the need for any additional UI customization.

@amvanbaren
Copy link
Contributor Author

@svor I tried your PR (#1057). The dynamic approach causes a slight glitch. The white screen is when I do a page refresh.

Here the isOAuth2Enabled initial value is false and the API returns true:

login-popin.webm

Here the isOAuth2Enabled initial value is true and the API returns true:

login-nochange.webm

the Publish and Login buttons would not be displayed in the UI by default

No, that would be a breaking change. open-vsx.org and other deployments expect the Publish and Login buttons to be displayed by default. Here the isOAuth2Enabled initial value is true and the API returns false. Does this work for you?

login-popout.webm

@svor
Copy link
Contributor

svor commented Dec 6, 2024

@amvanbaren I've answered in my PR #1057 (comment), please take a look.

@amvanbaren amvanbaren merged commit 8bffeb0 into eclipse:master Dec 9, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants