-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
ui: Add more explanatory texts for empty states #12354
Conversation
{{t "routes.dc.services.instance.upstreams.tproxy-mode.footer"}} | ||
</Action> | ||
</p> | ||
{{t "routes.dc.services.instance.upstreams.tproxy-mode.footer"}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these two might need htmlSafe=true
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh great catch! Lemme fix that up now 👍
ui/packages/consul-ui/app/templates/dc/services/instance/exposedpaths.hbs
Show resolved
Hide resolved
This has a little conflict now, likely to be done after the weekend will ping properly once its ready again |
ff9f5a6
to
cd4fb2a
Compare
😬 YAML conflicts are far more hairy than they should be. I have a few improvements in the works that I've not been able to PR as yet for various reasons that would help here. Pretty sure this one is correct now, looking at the diff here there was a bit of moving around of yaml-ness. @natmegs I guess it's ready for review now |
🎉 Thanks for the approval! Just a quick note here, there are a couple of questions outstanding on the textual content here so just gonna leave this hanging a little more until we get clarity on that. We may just merge and follow up with an additional PR or just pop more on that end here. Following up offline. |
Just to follow up here, I noticed the test we have added here might not be correct, if I've understood correctly the text we are using here only specifies that you need
So I've checked in with the team to see whether to merge this or wait until we get corrected text (if it does need correcting) |
7a4d770
to
b7d701d
Compare
I tried this out and Let me know if anything else needs to happen here, otherwise I'll leave it hanging for a little while longer then merge. |
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/632517. |
🍒✅ Cherry pick of commit e622756 onto |
* ui: Add more explanatory texts for empty states * Change all template "Read the guide"s * Add missing htmlSafe * Remove the stuff I commented out to try and grok the hairy rebase * Changelog * More rebased yaml weirdness plus added node:read
Related: #10426 (comment)
This PR does two things:
Small note here:
I noticed whilst doing this that we seem to be very close to full i18n'ed text in all our Routes now. In fact we are probably at a stage where we should make a move to specifically go ahead and make sure all our Routes are 100% i18n'ed seeing as we must be about 95% there, we may aswell go ahead and get the last 5% done soon-ish.