-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat(job): configurable title #1441
Conversation
- Use eds typography, reduce hierarchy size on title - Make parent space items, not individually - Re-insert the plugin wrapper (created duplicate padding) - Remove unecessary css classes
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.
Nice! 💯
I added a commit with some css cleanup if that's okay. dm-plugin-wrapper class gives us the opportunity to create spacing around plugins so combining it with the plugin wrapper created duplicate padding when it wasn't in Grid (like the recurring job example) so it might not have been that noticeable. I just reintroduced It and cleaned up some unnecessary css classes. Also used the EDS Typography component. For text it's fine to use tailwind/nothing, but headings are basically unstyled after tailwind reset, so we either need to use EDS typography (which is probably what we should do) or tailwind classes 🙂
Just a few questions 🙂 If 'dm-plugin-padding' is a "common plugin padding class", why can you not combine it with other classNames? Also, Equinor Typograpy is fine, but what is the big benefit? Would it not be better to just let the imported equinor-fonts in index.html set the fonts? Would make it easier to change later also. The result is the same anyhow |
What does this pull request change?
Adds a optional "title" to jobPluginConfig and UI
Why is this pull request needed?
Be able to describe what the "play button" does
Issues related to this change
closes #1418