Add package setting to toggle full-width status-bar #152

Merged
merged 4 commits into from Oct 12, 2016

Conversation

Projects
None yet
@Alhadis
Contributor

Alhadis commented Jun 7, 2016

I have writer's block, so I'll just echo @simurai's words, because he put it best:

But somehow it feels wrong. I think the reason is because the status-bar changes based on the active editor and should somewhat stay connected to it. Full-width at the bottom it feels like it's something more global and less related to the active editor.

I agree wholeheartedly, and some other users might too. Artistic rationale aside, stubborn and change-resilient users like me would appreciate having a setting to restore the status-bar's old appearance:

Figure 1

If it needs to be said, the setting's default value is to use the full-width status bar:

Figure 2

This is how the option appears in the package's settings:

Figure 3

@simurai

This comment has been minimized.

Show comment
Hide comment
@simurai

simurai Jun 7, 2016

Member

Here another thought: Separate the status-bar items based on if something is permanent (global) or just temporary (per file):

screen shot 2016-06-07 at 4 42 17 pm

Then the permanent items, like updates etc. could be on the left and under the "global" tree-view. On the right are all the items that change "per file". So the file name would be under the editor and look less disconnected..

..of course this falls flat for people that moved the tree-view to the right. And it might also be something that not all people like and demand the old the position back. 💭

Member

simurai commented Jun 7, 2016

Here another thought: Separate the status-bar items based on if something is permanent (global) or just temporary (per file):

screen shot 2016-06-07 at 4 42 17 pm

Then the permanent items, like updates etc. could be on the left and under the "global" tree-view. On the right are all the items that change "per file". So the file name would be under the editor and look less disconnected..

..of course this falls flat for people that moved the tree-view to the right. And it might also be something that not all people like and demand the old the position back. 💭

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Jun 7, 2016

Contributor

... what we have now works perfectly fine. There's no point changing what we're already used to.

I believe many users made that point abundantly clear when v1.7.0 was released.

Contributor

Alhadis commented Jun 7, 2016

... what we have now works perfectly fine. There's no point changing what we're already used to.

I believe many users made that point abundantly clear when v1.7.0 was released.

@pradyunsg

This comment has been minimized.

Show comment
Hide comment
@pradyunsg

pradyunsg Jun 7, 2016

I do think what has been proposed in this PR is the best compromise in the short-term.

@simurai has a nice idea. It might be a little too late to change the entire arrangement of the status-bar though.

But if someone comes up with a clean proposal for how this comes together (including not breaking the existing status-bar API), I'm all for it. It would be nice to have some such organization in the status bar.

of course this falls flat for people that moved the tree-view to the right.

No it doesn't. Just determine which side the tree-view is on (I'm assuming it's not too hard, from what I know of Atom's API) and switch sides.

pradyunsg commented Jun 7, 2016

I do think what has been proposed in this PR is the best compromise in the short-term.

@simurai has a nice idea. It might be a little too late to change the entire arrangement of the status-bar though.

But if someone comes up with a clean proposal for how this comes together (including not breaking the existing status-bar API), I'm all for it. It would be nice to have some such organization in the status bar.

of course this falls flat for people that moved the tree-view to the right.

No it doesn't. Just determine which side the tree-view is on (I'm assuming it's not too hard, from what I know of Atom's API) and switch sides.

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Jun 7, 2016

Contributor

Drag-and-drop might be nice.

Contributor

Alhadis commented Jun 7, 2016

Drag-and-drop might be nice.

@pradyunsg

This comment has been minimized.

Show comment
Hide comment
@pradyunsg

pradyunsg Jun 7, 2016

@simurai I think your idea is worth exploring but is still fairly disconnected to what is there in this PR.

Maybe open a new issue for discussing the separation of permanent and temporary status-bar items?

pradyunsg commented Jun 7, 2016

@simurai I think your idea is worth exploring but is still fairly disconnected to what is there in this PR.

Maybe open a new issue for discussing the separation of permanent and temporary status-bar items?

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Jun 7, 2016

Contributor

On it.

I'll bring it up in discuss.atom.io though, because the Atom team are swamped with enough issues as it is.

Done!

Contributor

Alhadis commented Jun 7, 2016

On it.

I'll bring it up in discuss.atom.io though, because the Atom team are swamped with enough issues as it is.

Done!

@gluons

This comment has been minimized.

Show comment
Hide comment
@gluons

gluons Jun 7, 2016

Wow! I really need this setting. Good news. 😄

gluons commented Jun 7, 2016

Wow! I really need this setting. Good news. 😄

@gluons gluons referenced this pull request Jun 7, 2016

Merged

Move to the footer #133

@50Wliu 50Wliu added the needs-review label Jun 7, 2016

@gh0stonio

This comment has been minimized.

Show comment
Hide comment

👍

@pierreneter

This comment has been minimized.

Show comment
Hide comment
@pierreneter

pierreneter Jul 3, 2016

e3c23e10-2cd3-11e6-821b-705518cb13ec

i cant see that option in my status-bar setting @.@

pierreneter commented Jul 3, 2016

e3c23e10-2cd3-11e6-821b-705518cb13ec

i cant see that option in my status-bar setting @.@

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Jul 4, 2016

Contributor

i cant see that option in my status-bar setting @.@

That's because this pull-request hasn't been merged yet...

Contributor

Alhadis commented Jul 4, 2016

i cant see that option in my status-bar setting @.@

That's because this pull-request hasn't been merged yet...

@50Wliu

This comment has been minimized.

Show comment
Hide comment
Member

50Wliu commented Jul 15, 2016

@simurai

This comment has been minimized.

Show comment
Hide comment
@simurai

simurai Jul 16, 2016

Member

I'm not so sure. It's nice to have the option, but it also adds extra complexity.

For example the One theme has a bottom border for the tree-view that should be removed if the status-bar isn't full-width. Maybe an extra class has-full-width-status-bar on the body could help, but it's another case that needs to be tested when making changes.

Is this a good candidate for the init.coffee?

@statusBar = document.querySelector('.status-bar')
atom.workspace.addBottomPanel(item: @statusBar, priority: 0)

And depending on the theme, might need a styles.less tweak.

Member

simurai commented Jul 16, 2016

I'm not so sure. It's nice to have the option, but it also adds extra complexity.

For example the One theme has a bottom border for the tree-view that should be removed if the status-bar isn't full-width. Maybe an extra class has-full-width-status-bar on the body could help, but it's another case that needs to be tested when making changes.

Is this a good candidate for the init.coffee?

@statusBar = document.querySelector('.status-bar')
atom.workspace.addBottomPanel(item: @statusBar, priority: 0)

And depending on the theme, might need a styles.less tweak.

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Jul 16, 2016

Contributor

For example the One theme has a bottom border for the tree-view that should be removed if the status-bar isn't full-width

What's wrong with moving the border to the status bar?

I'm also not sure where or how it adds complexity...

Contributor

Alhadis commented Jul 16, 2016

For example the One theme has a bottom border for the tree-view that should be removed if the status-bar isn't full-width

What's wrong with moving the border to the status bar?

I'm also not sure where or how it adds complexity...

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Jul 16, 2016

Contributor

BTW, the tree-view has a package setting to control its position in the workspace as well:

Figure 1

Since it's (physically) connected to the status bar, it feels logical that the status bar would have a similar layout-influencing setting as well.

Personally, I really think init.coffee shouldn't be a dumping ground for every setting not expected to see common use. If we follow that train of thought, it'd going to lead to some very bloated startup scripts...

Packages define a nice, neat space for holding package-specific settings, so why not make the most of it? =)

Contributor

Alhadis commented Jul 16, 2016

BTW, the tree-view has a package setting to control its position in the workspace as well:

Figure 1

Since it's (physically) connected to the status bar, it feels logical that the status bar would have a similar layout-influencing setting as well.

Personally, I really think init.coffee shouldn't be a dumping ground for every setting not expected to see common use. If we follow that train of thought, it'd going to lead to some very bloated startup scripts...

Packages define a nice, neat space for holding package-specific settings, so why not make the most of it? =)

@ajmichaels

This comment has been minimized.

Show comment
Hide comment
@ajmichaels

ajmichaels Jul 17, 2016

👍 As someone who toggles the atom-linter notifications quite often, the new position is fairly annoying.

👍 As someone who toggles the atom-linter notifications quite often, the new position is fairly annoying.

@lorenzos

This comment has been minimized.

Show comment
Hide comment
@lorenzos

lorenzos Jul 18, 2016

👍 +1 Lint issues notifications are invisible for me, now that they are below the tree view!

lorenzos commented Jul 18, 2016

👍 +1 Lint issues notifications are invisible for me, now that they are below the tree view!

@simurai

This comment has been minimized.

Show comment
Hide comment
@simurai

simurai Jul 24, 2016

Member

I'm not entirely against this option, because options are nice to have. Just that in general, adding config options can increase complexity because you have to account for different situations and can't assume certain things anymore. And it might add an extra burden to package/theme authors to test against all the different options.

For example it's nice that the tree-view can be moved to the right, but then the notifications cover the tree-view atom/notifications#93 which isn't ideal for some.

I really think init.coffee shouldn't be a dumping ground for every setting not expected to see common use.

Yeah, it's hard to guess what is considered "common use". So I wanted to make sure that:

  1. There aren't any side effects that we haven't thought about.
  2. This option would have enough user adoption to warrant the cost of maintaining an extra setting.
Member

simurai commented Jul 24, 2016

I'm not entirely against this option, because options are nice to have. Just that in general, adding config options can increase complexity because you have to account for different situations and can't assume certain things anymore. And it might add an extra burden to package/theme authors to test against all the different options.

For example it's nice that the tree-view can be moved to the right, but then the notifications cover the tree-view atom/notifications#93 which isn't ideal for some.

I really think init.coffee shouldn't be a dumping ground for every setting not expected to see common use.

Yeah, it's hard to guess what is considered "common use". So I wanted to make sure that:

  1. There aren't any side effects that we haven't thought about.
  2. This option would have enough user adoption to warrant the cost of maintaining an extra setting.
@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Jul 24, 2016

Contributor

There aren't any side effects that we haven't thought about.

Honestly, I can't see this introducing any issues. Even in uncommon scenarios:

  1. An author needs to apply specific styling to the status bar when contained in a certain pane:
    Simple CSS fix: .panel-bottom status-bar, .panel-footer status-bar.
  2. Package authors need to query or detect changes to status bar position:
    Atom's config API already covers that:
atom.config.get("status-bar.fullWidth");
atom.config.observe("status-bar.fullWidth");

I've also added specs to guard against regressions, just in case.

This option would have enough user adoption to warrant the cost of maintaining an extra setting.

At least seven other users who've seen this pull request have given a 👍, and a few others have even voiced their approval.

Contributor

Alhadis commented Jul 24, 2016

There aren't any side effects that we haven't thought about.

Honestly, I can't see this introducing any issues. Even in uncommon scenarios:

  1. An author needs to apply specific styling to the status bar when contained in a certain pane:
    Simple CSS fix: .panel-bottom status-bar, .panel-footer status-bar.
  2. Package authors need to query or detect changes to status bar position:
    Atom's config API already covers that:
atom.config.get("status-bar.fullWidth");
atom.config.observe("status-bar.fullWidth");

I've also added specs to guard against regressions, just in case.

This option would have enough user adoption to warrant the cost of maintaining an extra setting.

At least seven other users who've seen this pull request have given a 👍, and a few others have even voiced their approval.

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Sep 18, 2016

Contributor

So... what's happening with this...?

Contributor

Alhadis commented Sep 18, 2016

So... what's happening with this...?

@blcarson

This comment has been minimized.

Show comment
Hide comment
@blcarson

blcarson Oct 1, 2016

FWIW this change is driving me seriously batty. Is there an easy way I can get these changes in now?

blcarson commented Oct 1, 2016

FWIW this change is driving me seriously batty. Is there an easy way I can get these changes in now?

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Oct 1, 2016

Contributor

Yep! cd to wherever you'd like to keep a forked project, and run this:

git clone https://github.com/Alhadis/status-bar.git
cd status-bar && git checkout width-setting
apm install && apm link -d .

CAVAET: This requires a project to be opened in development mode in order to take effect.

Which is what I've been doing every day for over two months now, and I'm getting kinda sick of it.

Contributor

Alhadis commented Oct 1, 2016

Yep! cd to wherever you'd like to keep a forked project, and run this:

git clone https://github.com/Alhadis/status-bar.git
cd status-bar && git checkout width-setting
apm install && apm link -d .

CAVAET: This requires a project to be opened in development mode in order to take effect.

Which is what I've been doing every day for over two months now, and I'm getting kinda sick of it.

@blcarson

This comment has been minimized.

Show comment
Hide comment
@blcarson

blcarson Oct 1, 2016

Ahh - you're a saint!

I installed it in my ~/.atom/packages folder and it seems to work without developer mode. Any reason why you have to run through that process daily?

blcarson commented Oct 1, 2016

Ahh - you're a saint!

I installed it in my ~/.atom/packages folder and it seems to work without developer mode. Any reason why you have to run through that process daily?

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Oct 1, 2016

Contributor

Erh, really? I wasn't aware Atom enabled you to "override" packages that way... to date, I've simply been repeating the steps described here. I'm not sure this is expected behaviour.

@simurai, any idea if this is a bug?

Contributor

Alhadis commented Oct 1, 2016

Erh, really? I wasn't aware Atom enabled you to "override" packages that way... to date, I've simply been repeating the steps described here. I'm not sure this is expected behaviour.

@simurai, any idea if this is a bug?

@blcarson

This comment has been minimized.

Show comment
Hide comment
@blcarson

blcarson Oct 1, 2016

Steps I took, in case it's helpful:

  • Cloned the repo to my ~/.atom/packages folder
  • Ran the commands you suggested
  • Opened atom in dev mode
  • Enabled your package (which seemed to turn on the default one as well)
  • Toggled the width option setting to off
  • Started atom in normal mode and settings persisted

I'm on Mac 10.12 if it makes any difference.

screenshot

blcarson commented Oct 1, 2016

Steps I took, in case it's helpful:

  • Cloned the repo to my ~/.atom/packages folder
  • Ran the commands you suggested
  • Opened atom in dev mode
  • Enabled your package (which seemed to turn on the default one as well)
  • Toggled the width option setting to off
  • Started atom in normal mode and settings persisted

I'm on Mac 10.12 if it makes any difference.

screenshot

@simurai

This comment has been minimized.

Show comment
Hide comment
@simurai

simurai Oct 6, 2016

Member

@Alhadis If you use apm link instead of apm link -d, then the package is available also in normal mode. The reason to use apm link -d while you work on a package is so that you can run both versions at the same time. A normal mode window with the published package and a dev mode window with your local package. Maybe not needed for all package, but if you mess something up locally, your normal mode window will still work as usual.

btw. the One themes got changed so that the borders wouldn't be an issue anymore.

Member

simurai commented Oct 6, 2016

@Alhadis If you use apm link instead of apm link -d, then the package is available also in normal mode. The reason to use apm link -d while you work on a package is so that you can run both versions at the same time. A normal mode window with the published package and a dev mode window with your local package. Maybe not needed for all package, but if you mess something up locally, your normal mode window will still work as usual.

btw. the One themes got changed so that the borders wouldn't be an issue anymore.

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Oct 6, 2016

Contributor

Well, I know that... but seeing a core package listed as both a "Community Package" and a "Core "Package" looked pretty erroneous to me. But I digress.

My main question is: what's this bar's status?

Contributor

Alhadis commented Oct 6, 2016

Well, I know that... but seeing a core package listed as both a "Community Package" and a "Core "Package" looked pretty erroneous to me. But I digress.

My main question is: what's this bar's status?

@simurai

This comment has been minimized.

Show comment
Hide comment
@simurai

simurai Oct 6, 2016

Member

My main question is: what's this bar's status?

Ya, it's a bit weird that it shows the atom avatar and the download counts for your linked package that isn't really published.

Member

simurai commented Oct 6, 2016

My main question is: what's this bar's status?

Ya, it's a bit weird that it shows the atom avatar and the download counts for your linked package that isn't really published.

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Oct 6, 2016

Contributor

Eyyyyupp, that'll teach me for trying to be punny with a vague wisecrack (status-bar = "bar's status"). It sounded funnier in my head, honest.

It was a fatigue-encrypted way of asking if this PR is close to being merged yet...

Contributor

Alhadis commented Oct 6, 2016

Eyyyyupp, that'll teach me for trying to be punny with a vague wisecrack (status-bar = "bar's status"). It sounded funnier in my head, honest.

It was a fatigue-encrypted way of asking if this PR is close to being merged yet...

@simurai

This comment has been minimized.

Show comment
Hide comment
@simurai

simurai Oct 6, 2016

Member

Ahh.. hehe.. 😂

Well, I'm fine with either. But we need at least somebody else to review the code.

Member

simurai commented Oct 6, 2016

Ahh.. hehe.. 😂

Well, I'm fine with either. But we need at least somebody else to review the code.

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Oct 6, 2016

Contributor

I'll send a Mars Bar to any Atom member who gets this and another light-and-easy PR reviewed this week. For real.

I say that on the grounds nobody is insane enough to send their address so a sleep-deprived Australian can order them a chocolate bar.

Contributor

Alhadis commented Oct 6, 2016

I'll send a Mars Bar to any Atom member who gets this and another light-and-easy PR reviewed this week. For real.

I say that on the grounds nobody is insane enough to send their address so a sleep-deprived Australian can order them a chocolate bar.

@blcarson

This comment has been minimized.

Show comment
Hide comment
@blcarson

blcarson Oct 6, 2016

I'll up the ante and will donate a case of your favorite candy bar (assuming its available on Amazon Prime) to the cause 🍫

blcarson commented Oct 6, 2016

I'll up the ante and will donate a case of your favorite candy bar (assuming its available on Amazon Prime) to the cause 🍫

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Oct 6, 2016

Contributor

⬆️ This guy. Right across it. 👍 👌

Contributor

Alhadis commented Oct 6, 2016

⬆️ This guy. Right across it. 👍 👌

@blcarson

This comment has been minimized.

Show comment
Hide comment
@blcarson

blcarson Oct 12, 2016

New version of status bar breaks the width setting.

New version of status bar breaks the width setting.

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Oct 12, 2016

Contributor

Not anymore.

Contributor

Alhadis commented Oct 12, 2016

Not anymore.

@blcarson

This comment has been minimized.

Show comment
Hide comment
@blcarson

blcarson Oct 12, 2016

You're a rock star - thank you!

You're a rock star - thank you!

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Oct 12, 2016

Contributor

@nathansobo and I both fixed the same typo, which is all that triggered the conflict.

Amazing.

Contributor

Alhadis commented Oct 12, 2016

@nathansobo and I both fixed the same typo, which is all that triggered the conflict.

Amazing.

@nathansobo nathansobo merged commit 9f425dc into atom:master Oct 12, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Oct 12, 2016

Contributor

Thanks @Alhadis. This looks great and it will be nice to have this option.

Contributor

nathansobo commented Oct 12, 2016

Thanks @Alhadis. This looks great and it will be nice to have this option.

@Alhadis

This comment has been minimized.

Show comment
Hide comment
@Alhadis

Alhadis Oct 12, 2016

Contributor

If I weren't in the midst of crippling depression, I'd crown this the happiest day of my life. 👍 Thanks, Nathan.

Contributor

Alhadis commented Oct 12, 2016

If I weren't in the midst of crippling depression, I'd crown this the happiest day of my life. 👍 Thanks, Nathan.

@Alhadis Alhadis deleted the Alhadis:width-setting branch Oct 12, 2016

@bronson

This comment has been minimized.

Show comment
Hide comment
@bronson

bronson Nov 10, 2016

Anyone know why I can't see this setting? Atom 1.12.0, status-bar 1.4.1. Would really like to use it!

(felt like including a screenshot for some reason...)

screen shot 2016-11-10 at 11 36 48 am

bronson commented Nov 10, 2016

Anyone know why I can't see this setting? Atom 1.12.0, status-bar 1.4.1. Would really like to use it!

(felt like including a screenshot for some reason...)

screen shot 2016-11-10 at 11 36 48 am

@Ben3eeE

This comment has been minimized.

Show comment
Hide comment
@Ben3eeE

Ben3eeE Nov 10, 2016

Member

Atom 1.12.0, status-bar 1.4.1.

It is available in Atom 1.13.

Member

Ben3eeE commented Nov 10, 2016

Atom 1.12.0, status-bar 1.4.1.

It is available in Atom 1.13.

@bronson

This comment has been minimized.

Show comment
Hide comment
@bronson

bronson Nov 10, 2016

Ah, that's an easy explanation. Thanks!

bronson commented Nov 10, 2016

Ah, that's an easy explanation. Thanks!

@pierreneter

This comment has been minimized.

Show comment
Hide comment
@pierreneter

pierreneter Nov 11, 2016

thanks! that's good news.

thanks! that's good news.

@blcarson

This comment has been minimized.

Show comment
Hide comment
@blcarson

blcarson Nov 11, 2016

It's in the latest beta now - working well so far.

It's in the latest beta now - working well so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment