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

chore(ui5-breadcrumbs): current location hybrid #9421

Merged
merged 1 commit into from
Aug 8, 2024

Conversation

PetyaMarkovaBogdanova
Copy link
Contributor

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova commented Jul 9, 2024

Regarding to recent feedback we move back the variant with breadcrumbs current item implemented as a text and kept the functionality to visualise it as a link if a href is passed to the last breadcrumbs item.

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova force-pushed the breadcrumbs-current-loc-text-link branch 4 times, most recently from 0e0c339 to 9b70d51 Compare July 10, 2024 15:22
@PetyaMarkovaBogdanova PetyaMarkovaBogdanova force-pushed the breadcrumbs-current-loc-text-link branch from 9b70d51 to aabf59f Compare July 15, 2024 15:09
@PetyaMarkovaBogdanova PetyaMarkovaBogdanova marked this pull request as ready for review July 16, 2024 11:10
Copy link
Contributor

@kineticjs kineticjs left a comment

Choose a reason for hiding this comment

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

only some minor comments, check below

packages/main/src/Breadcrumbs.ts Outdated Show resolved Hide resolved
packages/main/src/Breadcrumbs.ts Show resolved Hide resolved
packages/main/src/Breadcrumbs.ts Outdated Show resolved Hide resolved
packages/main/src/Breadcrumbs.ts Outdated Show resolved Hide resolved
@PetyaMarkovaBogdanova PetyaMarkovaBogdanova force-pushed the breadcrumbs-current-loc-text-link branch from aabf59f to 2979d2d Compare July 17, 2024 14:19
@PetyaMarkovaBogdanova PetyaMarkovaBogdanova force-pushed the breadcrumbs-current-loc-text-link branch from 2979d2d to a3a305d Compare July 18, 2024 07:42
@dobrinyonkov
Copy link
Contributor

dobrinyonkov commented Jul 31, 2024

Related to: #8853

@dobrinyonkov
Copy link
Contributor

Removing the href of the last item during runtime has no effect on the last item. Rerendering is triggered, though.

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova force-pushed the breadcrumbs-current-loc-text-link branch from a3a305d to 163632a Compare August 1, 2024 12:19
@PetyaMarkovaBogdanova
Copy link
Contributor Author

Removing the href of the last item during runtime has no effect on the last item. Rerendering is triggered, though.

done, I was wrongly checking for indifference with only undefined falsy value, when the href resulted in a null one.

@PetyaMarkovaBogdanova PetyaMarkovaBogdanova force-pushed the breadcrumbs-current-loc-text-link branch from 163632a to fe86283 Compare August 8, 2024 07:56
@PetyaMarkovaBogdanova PetyaMarkovaBogdanova merged commit fe522a9 into main Aug 8, 2024
9 of 10 checks passed
@nnaydenow nnaydenow deleted the breadcrumbs-current-loc-text-link branch October 24, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants