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 page title sentence casing #2067

Merged
merged 1 commit into from
Nov 7, 2023

Conversation

dgutride
Copy link
Contributor

@dgutride dgutride commented Nov 3, 2023

Description

Changed three page titles to match the navigation item casing. Top level items should be Title Case, all nested items should be Sentence case.

Cluster settings (was Cluster Settings), User management (instead of User and group settings) and Data Science Projects (instead of Data science projects since it's a top level item) were all changed.

How Has This Been Tested?

Ran the site locally, loaded all impacted pages and ensured that the page title now matches the navigation item on the left hand side.

Test Impact

Ran storybook and other tests before putting up PR - none needed adjustment.

Request review criteria:

Self checklist (all need to be checked):

  • The developer has manually tested the changes and verified that the changes work
  • Commits have been squashed into descriptive, self-contained units of work (e.g. 'WIP' and 'Implements feedback' style messages have been removed)
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has added tests or explained why testing cannot be added (unit tests & storybook for related changes)

If you have UI changes:

  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change (find relevant UX in the SMEs section).

After the PR is posted & before it merges:

  • The developer has tested their solution on a cluster by using the image produced by the PR to main

Closes: #1907

@lucferbux
Copy link
Contributor

🎉🎉🎉🎉
@dgutride We might want to tag @kywalker-rh and @kaedward, in case they want to be aware and maybe we can update future mockups with this labels.
Tested locally and works great, I'll wait to UX first before lgtm it

@andrewballantyne
Copy link
Member

@dgutride the template asks for you to tag a UX person if there are UI changes... but in this case there is already a UX decision made... but we should always reping them to make sure we delivered on their ask. Lucas tagged Katie.

@andrewballantyne
Copy link
Member

Also @dgutride ... From a scan of the code, you're missing Model Serving. I'm spinning up a Cluster Bot cluster to make sure I can get a state in there somehow to see the page. Looks like Model Serving is having some issues on our shared cluster.

@dgutride
Copy link
Contributor Author

dgutride commented Nov 6, 2023

Screenshot 2023-11-06 at 9 04 15 AM
@andrewballantyne - fixed model serving to match (Title Case since it's a top level item). Removed validation in context so I could see this and took a screenshot.

@andrewballantyne
Copy link
Member

So this looks good... my only question is for @kaedward... what about breadcrumbs? It's not often we have them, but:

image
image

@dgutride we squash commits too -- so an FYI we will need to get you down to one commit before we merge.

@kaedward
Copy link

kaedward commented Nov 7, 2023

@andrewballantyne Breadcrumbs should match the casing of the navigation item and the project/item's casing - so "Data Science Projects > Andrew's test" (If that's how the project name is capitalized)

@andrewballantyne
Copy link
Member

@dgutride there is another fix you need to do -- there are a few breadcrumb values that need to be updated as well to fit the page naming. There are two "Data science projects" items for breadcrumbs that need to be updated and I think actually there is one for Data Science Pipelines that I just called "Projects" which needs to be updated too.

If you search for these things you should be able to find all 3 easily:

  • >Data science projects< (should have 2 results)
  • >Projects< (should have 1 result)

@dgutride
Copy link
Contributor Author

dgutride commented Nov 7, 2023

Fixed Data Science Project breadcrumbs, too - here's a few screenshots:
Screenshot 2023-11-07 at 1 41 22 PM

Screenshot 2023-11-07 at 1 41 59 PM

@dgutride
Copy link
Contributor Author

dgutride commented Nov 7, 2023

Screenshot 2023-11-07 at 1 44 33 PM

Copy link
Member

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

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

Looks good... We have some bugs in the way Pipelines references items inside a project vs the global page, but that's definitely separate of this initiative. I'll look to log something soon.

Copy link
Contributor

openshift-ci bot commented Nov 7, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved label Nov 7, 2023
@openshift-merge-bot openshift-merge-bot bot merged commit bcb243e into opendatahub-io:main Nov 7, 2023
jstourac added a commit to jstourac/ods-ci that referenced this pull request Dec 10, 2023
These changes are based on this fix into ODH-Dashboard component [1,2].

Note: there is still an inconsistency for `Model Serving` menu item and
relevant page title `Deployed models` which is gonna be handled by [3].

* [1] opendatahub-io/odh-dashboard#2067
* [2] opendatahub-io/odh-dashboard#1907
* [3] opendatahub-io/odh-dashboard#2105
jstourac added a commit to red-hat-data-services/ods-ci that referenced this pull request Dec 11, 2023
These changes are based on this fix into ODH-Dashboard component [1,2].

Note: there is still an inconsistency for `Model Serving` menu item and
relevant page title `Deployed models` which is gonna be handled by [3].

* [1] opendatahub-io/odh-dashboard#2067
* [2] opendatahub-io/odh-dashboard#1907
* [3] opendatahub-io/odh-dashboard#2105
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Page titles slightly inconsistent in text and case wrt other pages and menu titles
4 participants