Admin: Display buttons text

[agriffard]

[deanmarcussen]

Text on all screen sizes (IMHO)

Moving to just icons was not a great idea.

It's broken the look and feel of 2 sites (for me, that I know of), where I need to render extra buttons in the actions zone. And the extra buttons don't suit having just an icon - they need a textual description of what they do, as they are quite custom.

[hishamco]

Moving to just icons was not a great idea.

Dean from UI/UX perspective using icons is a good idea especially in the large screens, to save some space, the content items page for instance was trouble ;) coz it have tons of buttons, now it got better looking

[agriffard]

Button Text on all screens:

Used a ta-action-button to be customized in custom admin theme if needed.

Will demo next time.

[Skrypt]

What is it with this "ta-" prefix that we use it? Also, I think it would be more precise to name it ta-action-button-text then for those who wants to make it display: none;

If you want people to be able to customize the entire button based on a style then you should add the class on the <button> element, not just the <span>. So there it would make sense to name it ta-action-button and then they could still hide the <span> "title/text" with a css selector.

.ta-action-button > span {
  display: none;
}

.ta-action-button > svg {
  display: none;
}

Would hide both the icon and it's text/title.

[agriffard]

Prefix ta for theAdmin.

[Skrypt]

Refresh the page for my previous edited comment.

[Skrypt]

Ok @jptissot will be mad at me. We had this discussion before. And we don't even need the ta-action-button. The functional tests uses .btn.publish selector for example. And here it's the same and it will be consistent with what we need for the CI functional tests.

[agriffard]

So what: Do you want btn-localizations, btn-edit, btn-view, btn-preview, btn-actions?

[Skrypt]

class="btn btn-sm text-info localizations"

Repeat the same pattern for the others.

Then the specific css selector will be :

.btn.localizations > span {
  display: none;
}

.btn.localizations > svg {
  display: none;
}

If they want to remove text from all buttons they can still do

.btn > span {
  display: none;
}

/* or in a specific zone with a wrapper */

.wrapper {
  .btn > span {
    display: none;
  }
}

[Skrypt]

Here you can see how we did this for the "Create tenant" button. We just added the "create" class.

[Skrypt]

Now, you will say. But I can't disable them all at once just for these content list buttons. Then, we need a wrapper class on the actual Content List page that says. Here, I'm on the content list page ; disable the icons. Here, I'm on the tenant list page ; disable the button texts.

This class could be added on the <ul class="list-group with-checkbox **content-list**"> element.
Or use the "related" class here <div class="col-lg-6 col-sm-12 related"> if you want this to be applied globally, assuming we are consistent with that layout.

[Skrypt]

So looking at these html list elements and we are not consistent. Which is a remaining task to do. I don't think this PR should cover this part.

[Skrypt]

There is one small UI glitch that I found in Widgets.

[agriffard]

[agriffard]

Demo today. Should it look like this?:

• Use btn- instead of text-
• Keep icons
• Localizations and Actions can be just icons with dropdown-toggle

[hishamco]

Showing text inside some buttons doesn't make sense compare to others

[agriffard]

[agriffard]

Without ellipsis:

[deanmarcussen]

Approving, I think we've got to a better place where we are meeting more peoples needs.

I still feel like we've broken the Tags and Meta zones a bit, but will have to see it in action again to understand.

Good to merge later to wait for other opinions /cc @Skrypt

[hishamco]

Without ellipsis:

I see the UI is inconsistent, if we want to diplay text with icons, we shall do it for all buttons, again regarding the colors it should be choosing properly with the respect of theme color scheme

[deanmarcussen]

The returning colors are the colors that existed previously.
Most of this pr is about returning behaviour that broke existing sites.

TODO On another PR. Fix the zone breaking changes

[scleaver]

Hey guys, I said this over on the tenants UI changes - "I am not a fan of too many icons for actions as they require interpretation by the user" and I was hoping the rest of the UI wasn't going this way #8015 (comment)

You guys put so much work into OC and I am grateful so I hate to complain but there is quite a few issues with these UI changes as someone who is using this for clients right now and who is building some SaaS stuff on the framework which will be impacted by these changes:

1. Firstly, the code base is meant to be on RC2... the projects own overview says "...tightly scoped bug fixes are the only code you're allowed to write in this phase..." this is not tightly scoped and it is causing more work for those who have already built stuff on OC. I have already had to drop TheAdmin's stylesheet because of issues with dark theme clashing with my own customization.
2. In my view if you want to introduce an icon heavy UI, leave TheAdmin as text based as @deanmarcussen mentioned (using BEM style classes like "btn--publish") and do a second IconAdmin theme allowing us who have already created admin themes using "TheAdmin" as the base to continue to do so. You can add icons without have an <i></i> HTML inside the button by creating .btn-publish::before {} and applying the fontawesome conde to the selector.
3. This makes the UI very busy when much of the industry is going down a minimalist road trying to de-clutter interfaces and surfacing only key information.
4. There are accessibility issues with the way the Tenant UI has been coded which I haven't had a chance to raise. Personally I am going to be going for a reasonable amount of accessibility because of the audience I am aiming for. Both the code and the icons are accessibility issues both from a cognitive and semantic point of view.
5. OC is a powerful framework and as such the project should be aiming towards maximum flexibility with as minimal hacking as possible so that developers can create whatever they need to. Because of the way these changes are being coded it is pretty hard to customise without hiding stuff using display: none; and/or even worse copying views into your own admin theme to just remove stuff. There are so many views it would be easier if the admin was code first coded so that most situations don't require replacing views. This can be achieved if the HTML output is good and clean.
6. Lastly - it's 7pm on Christmas Eve in Australia and I am in trouble by my wife for being on the computer but this is too big a change not to way in on.

Some semantic thoughts:

• <a></a> tag should only be used if it is actually a link (like the link to the edit view of the content item)... <button></button> should be used if it is an action.
• Always always have text. This is not good: <a edit-for="@contentItem" asp-route-returnUrl="@FullRequestPath" data-toggle="tooltip" title="@T["Edit"]" class="btn btn-sm text-primary"><i class="fa fa-edit"></i></a> rather it should be <a edit-for="@contentItem" asp-route-returnUrl="@FullRequestPath" data-toggle="tooltip" title="@T["Edit"]" class="btn btn-sm text-primary btn--edit">@T["Edit"]</a> then apply the icon via .btn--edit::before { content: 'unicode'; font-family: fontawesome; etc etc }. If you create a mixin for fontawesome you could achieve this

[agriffard]

Thank you for raising your concerns @scleaver.
I think they will be adressed as this PR brings back the text of the buttons, right?

[agriffard]

If many people are against icons, I guess we can remove them.
In this case, please add a 👍 to @scleaver 's comment: #8086 (comment)

[agriffard]

Ok, no more icons.
Screenshot updated in first comment.

[Skrypt]

I have already had to drop TheAdmin's stylesheet because of issues with dark theme clashing with my own customization.

@scleaver Please reach to me directly on Skype so that I can look at customizations you did. I can look how to make them work and keep using the new stylesheets. I'm really sorry for your time loss on this but I think I explained why we moved to this strict stylesheets.

BEM styles have nothing really to do with that. It's fine using this naming approach and sometimes it's not. It should not break your customizations if they are declared or compiled in the proper place. I have myself too an admin theme with some customizations and I copied over all the scss styles to my custom admin theme to add my own styles and to be able to "transpile" them. Another option would be to be able to import these scss files from a node_module npm package like I said in a different issue comment. Though, I did not have time yet to do so. (iteration madness)
For stylesheets I think there is a none issue because there is a workaround.

Though, I have to agree that here we are moving items from one zone to an other which will break the custom admin UI changes that you made. Nothing related with CSS. These are DOM element changes which can't be fixed other than moving these element back to their corresponding zones.

Removing the icons was not suggested in the last meeting even though using stylesheets would be a more appropriate way to be able to customize this for your own needs. I think @sebastienros was clear about making these back to buttons and that we should be deterministic in our approach to do styles for the Admin UI. If people wants to do customizations on these we added a class for each of these buttons to style them ; which covers the need to extend them. I suggested in the last meeting to keep at least the text on each of these buttons and hide them with CSS for those that we don't need to show by default like "Localization" button. But this was not taken into consideration.

So, in the end, if you want to do some customization on the Admin UI, right now, this PR won't fix anything for you. It will break your UI and it means that if you did customizations then it's on you. Sorry to say. I'm trying to keep things extensible at least but it will require some more iterations and compromises from everyone. Also, any UI changes might introduce some breaking changes at some point if we want to move forward.

If there are any issues with this PR, please report it and we will fix it in priority.

Thank you.

Merry Christmas.