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

fix: Update menu switch CSS selectors #1477

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Update menu switch CSS selectors #1477

wants to merge 1 commit into from

Conversation

enjeck
Copy link
Contributor

@enjeck enjeck commented Nov 27, 2024

Once again, the CSS styling changed and we have to update this. Otherwise, we have the error message in the console: "TypeError: Cannot read properties of null (reading 'classList')"

Would be nice to someday have a better solution for this, since we've had to update these styles several times already

Signed-off-by: Cleopatra Enjeck M. <[email protected]>
@enjeck enjeck added bug Something isn't working 3. to review Waiting for reviews labels Nov 27, 2024
@enjeck enjeck requested a review from juliusknorr November 27, 2024 07:54
@enjeck enjeck self-assigned this Nov 27, 2024
@enjeck enjeck requested a review from blizzz as a code owner November 27, 2024 07:54
@@ -82,20 +82,20 @@ export default {
this.$store.commit('setActiveTableId', parseInt(currentRoute.params.tableId))
this.setPageTitle(this.$store.getters.activeTable.title)
if (!currentRoute.path.includes('/row/')) {
this.switchActiveMenuEntry(document.querySelector('header .header-left .app-menu a[title="Tables"]'))
this.switchActiveMenuEntry(document.querySelector('header .header-start .app-menu a[title="Tables"]'))
Copy link
Member

Choose a reason for hiding this comment

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

I think this change also only happened on master/stable30 not older versions so we might need a fallback.

@juliusknorr
Copy link
Member

Would be nice to someday have a better solution for this, since we've had to update these styles several times already

Yes, definitely a good idea. The app menu in server has some API to update or set a counter already, maybe we can think about extending that to set the active entry:

https://github.com/nextcloud/server/blob/c5baf3d7ef2715def79ff7734d505c56bb20012c/core/src/components/MainMenu.js#L29
https://github.com/nextcloud/server/blob/21c30e52591f6e73f28bf7dc5d9584f609f911f7/core/src/components/AppMenu.vue#L89-L95

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working
Projects
Status: 🏗️ In progress
Development

Successfully merging this pull request may close these issues.

2 participants