-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
two additional validator tools #131
base: main
Are you sure you want to change the base?
Conversation
feature: regex-validator for local regex texting
name: 'PZN validator', | ||
component: <PznValidator />, | ||
category: 'Validate', | ||
description: 'Validates PZN (8-digits) from german pharmaceutical system', |
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.
Sorry but I think this is not very widely used developer tool. In case you need this in your backstage instance, you can create the tool in your own repo and pass it to additionalTools
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 can split that off. Let me see, what I can do.
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.
pzn-validator is dropped.
{ | ||
id: 'regex', | ||
name: 'Regex validator', | ||
component: <RegexValidator />, |
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.
This on the other hand is very much wanted feature! Thanks!
const regex = new RegExp(patternString, flags); | ||
const testResults = regex.exec(inputString); | ||
if (testResults === null) { | ||
return "pattern doesn't match the input text"; |
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.
Please use translations for all messages to user
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.
translated
let result = ''; | ||
for (let i = 0; i < testResults.length; ++i) { | ||
if (testResults[i].length === 0) { | ||
result += "no match or empty!"; |
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.
Translations here as well
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.
translated
// regex check | ||
outputString = isRegexValid(input, pattern); | ||
} catch (error) { | ||
outputString = `there was an exception while parsing your regex\n\nerrormessage=${error.message}`; |
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.
And translations
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.
translated
color="inherit" | ||
sx={{ | ||
...(mode === m && { | ||
color: '#000000', |
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.
All colors and styles should come from theme, these might not work in every instance of Backstage with custom themes
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 only copied the DefaultEditor and enhanced it, actually I got no clue, how to have theming... must be done later, when I know how to...
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.
Ok, I can take a look after this is merged!
size="small" | ||
disableElevation | ||
variant="contained" | ||
aria-label="Disabled elevation buttons" |
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.
Translations in use here
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.
translated
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.
Hm, you should use the const { t } = useToolboxTranslation();
for this as well
enhanced: regex-validator with translation feature: german translation
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.
One missing translation but otherwise looks nice! I can check the styling after this is merged; anyways it needs some refactoring to allow overriding some of the styles in the App.
import { createTranslationMessages } from '@backstage/core-plugin-api/alpha'; | ||
import { toolboxTranslationRef } from '../translation'; | ||
|
||
const de = createTranslationMessages({ |
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.
Awsome!
color="inherit" | ||
sx={{ | ||
...(mode === m && { | ||
color: '#000000', |
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.
Ok, I can take a look after this is merged!
size="small" | ||
disableElevation | ||
variant="contained" | ||
aria-label="Disabled elevation buttons" |
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.
Hm, you should use the const { t } = useToolboxTranslation();
for this as well
Some formatting errors in lint, otherwise looks good! |
Ok. Got no clue, what 'aria-label' does as parameter for this element, it was copied before from DefaultEditor class/file. It is not represented in original translation-page, so I left this as is. - The color-scheming and the lint-checks are totally unknown for me. I have to look for them later, sorry - I'm a java-developer which has never done react-coding up to this time before. |
Tsc fails |
- worked on yarn tsc messages
feature: pzn-validator for german pharmacy codes
feature: regex-validator for local regex texting