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

Display attributes with correct capitalization #494

Closed
CaroFG opened this issue Apr 22, 2024 · 7 comments · Fixed by #547
Closed

Display attributes with correct capitalization #494

CaroFG opened this issue Apr 22, 2024 · 7 comments · Fixed by #547
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@CaroFG
Copy link

CaroFG commented Apr 22, 2024

Description

Attributes are uppercased. It's prettier but it can be confusing. Recently, a free-trial customer was getting 0 search results, because he had copied the searchable attributes from the mini-dashboard.

As @guimachiavelli pointed out:

You could argue meilisearch/documentation#2 is not super necessary, but it still seems like an easy enough thing to do and might serve as a subtle way of signalling capitalization is important.

Expected behavior

Attributes should be shown as they were sent, with the original case.

Current behavior

They are uppercased.

Screenshots or Logs
If applicable, add screenshots or logs to help explain your problem.

Environment (please complete the following information):

  • Meilisearch version: [e.g. v.0.24.0]
  • Browser: [e.g. Chrome version 90.0]
@curquiza
Copy link
Member

Thanks
I put this issue as good first issue for the community. Feel free to open a PR

@tacman
Copy link
Contributor

tacman commented Oct 8, 2024

@curquiza Can you point to the file where this happens? Indeed, it's a good first issue, as it's almost certainly removing something. I looked for :::to_ascii_uppercase but didn't find it.

@curquiza
Copy link
Member

curquiza commented Oct 8, 2024

Pinging @mdubus when you have time to guide the user please 😊

@mdubus
Copy link
Member

mdubus commented Oct 8, 2024

Hey @tacman 👋

Thanks for your interest in this repo 🙏

The file you're looking for is the src/components/Results/Hit.js one. You can see in the file that it has a HitKey component. The HitKey has a typo10 variant which has the text-transform: uppercase css class, causing our issue.

I guess the way to resolve it would be to override this class to have a text-transform: none here instead (I see in the codebase that it's used elsewhere so we don't want to break anything by removing it).

Let me know if you have any questions, I'd be happy to help you with it 😊

@tacman
Copy link
Contributor

tacman commented Oct 8, 2024

Only Donald Trump thinks that all caps is easier to read. I think the css in type10 should be changed.

It appears that the only other place it's used is in the Version string. Personally, I think v1 is easier to read than V1, and in fact it's overwritten there too.

I've made a PR that mimics how the version is rendered, but since the only 2 places I can find type10 overrides the text transformation, it would make more sense to remove it in the css. But perhaps these type definitions are used elsewhere.

@mdubus
Copy link
Member

mdubus commented Oct 8, 2024

@tacman indeed I just checked and it would make more sense to just remove the text-transform from the CSS.
I've seen you've opened a PR, could you make the change there? You can also remove the override in the header too 👍

Thanks for your help 🙌 🙏

tacman added a commit to tacman/meili-mini-dashboard that referenced this issue Oct 8, 2024
@tacman
Copy link
Contributor

tacman commented Oct 8, 2024

Done!

@meili-bors meili-bors bot closed this as completed in 155f4f7 Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants