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

[Bugfix]: Titles of pages (dashboard, apps-settings, users-settings) #43577

Merged
merged 3 commits into from
Feb 26, 2024

Conversation

arublov
Copy link
Member

@arublov arublov commented Feb 14, 2024

Summary

  1. Change logic in title, added the condition:
  • print application name only if it's equal to the page title
  1. Changed on Settings/App from (Apps - Apps - Nextcloud) to (Settings - Apps - Nextcloud)
  2. Changed on Users from (Users - Users - Nextcloud) to (Settings - Users - Nextcloud)

Checklist

@arublov
Copy link
Member Author

arublov commented Feb 14, 2024

@emoral435 and @szaimen ,

Done ✅ Please review :)

@arublov arublov changed the title [Bugfix]: Titles of pages (dashboard, apps-settings, users-settings) #43327 [Bugfix]: Titles of pages (dashboard, apps-settings, users-settings) Feb 14, 2024
@szaimen szaimen added this to the Nextcloud 29 milestone Feb 14, 2024
@szaimen szaimen added 3. to review Waiting for reviews bug labels Feb 14, 2024
Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

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

Tested. and works on local dev env! Dashboard (main issue in the OG PR for a11y team) has correct semantic labeling, and so does all other templates / views 👍

Big thank you for the contribution from the team over at Nextcloud 💙

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

🐘

@arublov
Copy link
Member Author

arublov commented Feb 21, 2024

@emoral435 / @szaimen , "Some checks were not successful" something happened on my side? Is there anything I can do to help with this?

@emoral435
Copy link
Contributor

@arublov Yes, of course!
So, from the looks of it, it seems that the PHP just need some linting, which further steps can be seen in the error messages these actions / checks throw out:

image

They detail what the script ran to complete the action and if the status came back as a fail code or a success code. In this case, for PHP lint, it seems that you just need to run the lint action on your local machine, fix the changes, rebase the code to get it updates, and push the changes.

For the cypress failures, since we made frontend changes, it is likely that they are related, but I tried running them again. If they are still not passing by the time the linting has been fixed, I will fix it.

Feel free to ping me if this happens! :) Anything for our contributors 🫡😤

auto-merge was automatically disabled February 23, 2024 17:39

Head branch was pushed to by a user without write access

Andrii Rublov added 2 commits February 23, 2024 18:39
Signed-off-by: Andrii Rublov <airublev@outlook.com>
@arublov
Copy link
Member Author

arublov commented Feb 23, 2024

@emoral435 , cypress - still with errors, PHP lint - I've run and updated this PR.

But the thing is, why?) the right version has more clean and structured code.
Screenshot 2024-02-23 at 6 40 15 PM

@skjnldsv
Copy link
Member

conflicts

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Feb 24, 2024
Signed-off-by: Andrii Rublov <github.a@rublov.me>
@arublov
Copy link
Member Author

arublov commented Feb 25, 2024

@skjnldsv , resolved

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Feb 26, 2024
@skjnldsv
Copy link
Member

@arublov thanks!
On an unrelated note, please favour rebase instead of merge to make your PR up to date again. That helps keeping the git history clean :)
image

@skjnldsv skjnldsv merged commit 00c53c9 into nextcloud:master Feb 26, 2024
150 of 158 checks passed
@arublov arublov deleted the fix/pages-titles-apps branch February 26, 2024 13:04
@blizzz blizzz mentioned this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: title of dashboard is "Dashboard - Dashboard - NextCloud"
4 participants