feat: change workspace names to icons #1240
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import { Component, ReactNode } from 'react'; | ||
| import { observer } from 'mobx-react'; | ||
| import withStyles, { WithStylesProps } from 'react-jss'; | ||
| import classnames from 'classnames'; | ||
| import { mdiInfinity } from '@mdi/js'; | ||
| import Icon from './icon'; | ||
|
|
||
| const styles = theme => ({ | ||
| root: { | ||
| height: `${theme.serviceIcon.width}px`, | ||
| width: `${theme.serviceIcon.width}px`, | ||
| textAlign: 'center', | ||
| display: 'table-cell', | ||
| verticalAlign: 'middle', | ||
| }, | ||
| icon: { | ||
| height: 'auto', | ||
| width: `${theme.serviceIcon.width}px`, | ||
| maxHeight: `${theme.serviceIcon.width}px`, | ||
| }, | ||
| iconLetterContainer: { | ||
| border: `1px solid ${theme.colorText}`, | ||
| borderRadius: '20%', | ||
| }, | ||
| iconLetter: { | ||
| fontSize: 'x-large', | ||
| fontWeight: '500', | ||
| }, | ||
| }); | ||
|
|
||
| interface IProps extends WithStylesProps<typeof styles> { | ||
| className?: string; | ||
| name: string | null; | ||
| iconUrl: string; | ||
| } | ||
|
|
||
| @observer | ||
| class WorkspaceIcon extends Component<IProps> { | ||
| render(): ReactNode { | ||
| const { classes, className, name, iconUrl } = this.props; | ||
| if (iconUrl === 'allServices') { | ||
| return ( | ||
| <div className={classnames([classes.root, className])}> | ||
| <Icon icon={mdiInfinity} size={1.5} /> | ||
| </div> | ||
| ); | ||
| } | ||
| if (iconUrl) { | ||
| return ( | ||
| <div className={classnames([classes.root, className])}> | ||
| <img src={iconUrl} className={classes.icon} alt={name || undefined} />{' '} | ||
| </div> | ||
| ); | ||
| } | ||
| return ( | ||
| <div | ||
| className={classnames([ | ||
| classes.root, | ||
| classes.iconLetterContainer, | ||
| className, | ||
| ])} | ||
| > | ||
| <span className={classes.iconLetter}> | ||
| {(name || ' ').slice(0, 1).toLocaleUpperCase()} | ||
| </span> | ||
| </div> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| export default withStyles(styles, { injectTheme: true })(WorkspaceIcon); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -263,6 +263,10 @@ const messages = defineMessages({ | |
| id: 'settings.app.form.alwaysShowWorkspaces', | ||
| defaultMessage: 'Always show workspace drawer', | ||
| }, | ||
| useWorkspaceDrawerIconStyle: { | ||
| id: 'settings.app.form.useWorkspaceDrawerIconStyle', | ||
| defaultMessage: 'Use workspace drawer icon style', | ||
|
There was a problem hiding this comment. So, I was suggesting to have the user choose whether they want the name or the first character of the workspace name AND/OR the icon. Could the feature be imagined like that? There was a problem hiding this comment. If I understand correctly. The letter is currently a fallback for workspaces that don't have an icon. The setting should be a select with theses choices :
It means, 3 different workspace drawer sizes
There was a problem hiding this comment. Should the workspace drawer bar be the same size of the service bar ? There was a problem hiding this comment. @vraravam Can you confirm that I have understood correctly ? There was a problem hiding this comment. we don't need to complicate so much. only 2 options that i was suggesting:
There was a problem hiding this comment. To permanently save this field you would probably need to sync with the server (internal and external) if I'm correct. I'm not recalling if the server is saving workspace data as a plain JSON or not (only by checking the endpoint we can be sure of that). If that is not the case, you would need to touch both the server and the internal-server... but with the ongoing Adonis update this could be a hassle :/ I can help on that tho. There was a problem hiding this comment. Also, can we make this option the default one? I like this new look (even though I'm not using workspaces). Great work!! There was a problem hiding this comment. yes, you will need to sync to the internal and external servers. The db schema is the same across both - and there's a table called As seen in the above pic (taken from my internal server), you can either add a new column for the There was a problem hiding this comment.
Even though I would prefer option 1 (just like you) I would advise into adding it to "data" column (but first lets be sure what type of data is being retrieved there) so that it doesn't make the migration to the new Adonis's even more complicated (as we would to update both the code in the current ferdium-server, internal server and on the current adonis update PR) There was a problem hiding this comment. As suggested, i added the iconUrl field in WIP related PR on the server : ferdium/ferdium-server#102 |
||
| }, | ||
| accentColor: { | ||
| id: 'settings.app.form.accentColor', | ||
| defaultMessage: 'Accent color', | ||
|
|
@@ -435,6 +439,9 @@ class EditSettingsScreen extends Component< | |
| hideSettingsButton: Boolean(settingsData.hideSettingsButton), | ||
| hideDownloadButton: Boolean(settingsData.hideDownloadButton), | ||
| alwaysShowWorkspaces: Boolean(settingsData.alwaysShowWorkspaces), | ||
| useWorkspaceDrawerIconStyle: Boolean( | ||
| settingsData.useWorkspaceDrawerIconStyle, | ||
| ), | ||
| accentColor: settingsData.accentColor, | ||
| progressbarAccentColor: settingsData.progressbarAccentColor, | ||
| showMessageBadgeWhenMuted: Boolean( | ||
|
|
@@ -1120,6 +1127,15 @@ class EditSettingsScreen extends Component< | |
| default: DEFAULT_APP_SETTINGS.alwaysShowWorkspaces, | ||
| type: 'checkbox', | ||
| }, | ||
| useWorkspaceDrawerIconStyle: { | ||
| label: intl.formatMessage(messages.useWorkspaceDrawerIconStyle), | ||
| value: ifUndefined<boolean>( | ||
| settings.all.app.useWorkspaceDrawerIconStyle, | ||
| DEFAULT_APP_SETTINGS.useWorkspaceDrawerIconStyle, | ||
| ), | ||
| default: DEFAULT_APP_SETTINGS.useWorkspaceDrawerIconStyle, | ||
| type: 'checkbox', | ||
| }, | ||
| accentColor: { | ||
| label: intl.formatMessage(messages.accentColor), | ||
| value: ifUndefined<string>( | ||
|
|
||
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.
Would it be better to move the ternary to be evaluated inside the
translateX()like so?(Syntax might be incorrect since I'm not typing inside an editor)
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.
Sure, we can do that