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

refactor(breadcrumb): replace 'strong' with 'breadcrumb' tags #3043

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

escii
Copy link
Collaborator

@escii escii commented Apr 28, 2024

@escii escii requested a review from tonyblank April 28, 2024 23:30
…tro-breadcrumb

# Conflicts:
#	astro/src/content/docs/_shared/_admin-user-form.mdx
#	astro/src/content/docs/_shared/_admin-user-registration-form.mdx
tonyblank
tonyblank previously approved these changes May 17, 2024
Copy link

@tonyblank tonyblank left a comment

Choose a reason for hiding this comment

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

LGTM - so many breadcrumbs, I feel like a duck!

@Aaron-Ritter
Copy link
Collaborator

Aaron-Ritter commented May 18, 2024

thanks for the review @tonyblank while merging the latest updates i found a mistake in a few changes we did where it should have been replaced with <UIelement></UIelement> instead.

@Aaron-Ritter
Copy link
Collaborator

@alex-fusionauth I've merged the latest changes and updated some newly introduced <strong> tags and we are ready to merge. As discussed, while reviewing for any other changes the single step navigation instructions. I've update all these accordingly as well with the <Breadcrumb> tag.

The few ones unclear right now are the ones where it refers to navigate to a section rather a full page/tab.

@Aaron-Ritter
Copy link
Collaborator

@alex-fusionauth defined the sections as a <InlineUIElement>

@andrewpai
Copy link
Contributor

andrewpai commented Jul 5, 2024

Here is a file of lines that in most cases need <Breadcrumb> instead of some other styling. A few need <InlineUIElement>.
breadcrumbs-needed.txt

@Aaron-Ritter
Copy link
Collaborator

Aaron-Ritter commented Jul 11, 2024

Here is a file of lines that in most cases need <Breadcrumb> instead of some other styling. A few need <InlineUIElement>. breadcrumbs-needed.txt

@andrewpai Working on this at the moment, and wanted to clarify, we will not only change the site content but all blog posts too with the same concept. And according to that i will update the blog section in DocsDevREADME.md with that guideline too.

@andrewpai
Copy link
Contributor

Here is a file of lines that in most cases need <Breadcrumb> instead of some other styling. A few need <InlineUIElement>. breadcrumbs-needed.txt

@andrewpai Working on this at the moment, and wanted to clarify, we will not only change the site content but all blog posts too with the same concept. And according to that i will update the blog section in DocsDevREADME.md with that guideline too.

Correct, we should make our docs consistent across /docs, /blog, and /articles.

…o-breadcrumb

# Conflicts:
#	astro/src/content/docs/lifecycle/authenticate-users/single-sign-on.mdx
@Aaron-Ritter
Copy link
Collaborator

Aaron-Ritter commented Jul 13, 2024

@andrewpai can we consider a read-only field still a InlineField or is InlineUIElement preferred? The most common case is the ID field which can be at different stages either of it.

And further specifying fields would be good too: can we define all types of form fields e.g. text, drop-down selection, checkbox considered InlineField, defining it as where ever info is being entered?

If it comes to the field input, it isn't standardized at the moment we have <strong>, *, **, `, ", and in few places just text at the moment. Could this be a new definition e.g. InlineFieldInput?

@andrewpai
Copy link
Contributor

@andrewpai can we consider a read-only field still a InlineField or is InlineUIElement preferred? The most common case is the ID field which can be at different stages either of it.

And further specifying fields would be good too: can we define all types of form fields e.g. text, drop-down selection, checkbox considered InlineField, defining it as where ever info is being entered?

If it comes to the field input, it isn't standardized at the moment we have \<strong\>, *, **, , "`, and in few places just text at the moment. Could this be a new definition e.g. InlineFieldInput?

Sorry, I completely missed this. InlineField was created so we could reference a field in an API request or response along with its various annotations, inline to some text vs. called out on its own line. I'm not sure if the read-only field fits that description, but if so then I think InlineField is appropriate. If it isn't a field used in an API then maybe InlineUIElement is better.

@Aaron-Ritter
Copy link
Collaborator

Aaron-Ritter commented Jul 30, 2024

Thanks @andrewpai,

We will go ahead and define a read-only form field as InlineField as it is still a form field.

- If you are referencing a field in a form or JSON API doc, use the [InlineField](astro/src/components/InlineField.astro) component: `<InlineField>Issuer</InlineField>`.

If it comes to the field input, we go ahead and use the <strong> tag everywhere unless you want to create a custom tag.

@escii escii requested review from a team as code owners November 25, 2024 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants